diff --git a/src/agents/tools/browser-tool.actions.ts b/src/agents/tools/browser-tool.actions.ts new file mode 100644 index 00000000000..95768891264 --- /dev/null +++ b/src/agents/tools/browser-tool.actions.ts @@ -0,0 +1,348 @@ +import type { AgentToolResult } from "@mariozechner/pi-agent-core"; +import { browserAct, browserConsoleMessages } from "../../browser/client-actions.js"; +import { browserSnapshot, browserTabs } from "../../browser/client.js"; +import { DEFAULT_AI_SNAPSHOT_MAX_CHARS } from "../../browser/constants.js"; +import { loadConfig } from "../../config/config.js"; +import { wrapExternalContent } from "../../security/external-content.js"; +import { imageResultFromFile, jsonResult } from "./common.js"; + +type BrowserProxyRequest = (opts: { + method: string; + path: string; + query?: Record; + body?: unknown; + timeoutMs?: number; + profile?: string; +}) => Promise; + +function wrapBrowserExternalJson(params: { + kind: "snapshot" | "console" | "tabs"; + payload: unknown; + includeWarning?: boolean; +}): { wrappedText: string; safeDetails: Record } { + const extractedText = JSON.stringify(params.payload, null, 2); + const wrappedText = wrapExternalContent(extractedText, { + source: "browser", + includeWarning: params.includeWarning ?? true, + }); + return { + wrappedText, + safeDetails: { + ok: true, + externalContent: { + untrusted: true, + source: "browser", + kind: params.kind, + wrapped: true, + }, + }, + }; +} + +function formatTabsToolResult(tabs: unknown[]): AgentToolResult { + const wrapped = wrapBrowserExternalJson({ + kind: "tabs", + payload: { tabs }, + includeWarning: false, + }); + const content: AgentToolResult["content"] = [ + { type: "text", text: wrapped.wrappedText }, + ]; + return { + content, + details: { ...wrapped.safeDetails, tabCount: tabs.length }, + }; +} + +function isChromeStaleTargetError(profile: string | undefined, err: unknown): boolean { + if (profile !== "chrome") { + return false; + } + const msg = String(err); + return msg.includes("404:") && msg.includes("tab not found"); +} + +function stripTargetIdFromActRequest( + request: Parameters[1], +): Parameters[1] | null { + const targetId = typeof request.targetId === "string" ? request.targetId.trim() : undefined; + if (!targetId) { + return null; + } + const retryRequest = { ...request }; + delete retryRequest.targetId; + return retryRequest as Parameters[1]; +} + +export async function executeTabsAction(params: { + baseUrl?: string; + profile?: string; + proxyRequest: BrowserProxyRequest | null; +}): Promise> { + const { baseUrl, profile, proxyRequest } = params; + if (proxyRequest) { + const result = await proxyRequest({ + method: "GET", + path: "/tabs", + profile, + }); + const tabs = (result as { tabs?: unknown[] }).tabs ?? []; + return formatTabsToolResult(tabs); + } + const tabs = await browserTabs(baseUrl, { profile }); + return formatTabsToolResult(tabs); +} + +export async function executeSnapshotAction(params: { + input: Record; + baseUrl?: string; + profile?: string; + proxyRequest: BrowserProxyRequest | null; +}): Promise> { + const { input, baseUrl, profile, proxyRequest } = params; + const snapshotDefaults = loadConfig().browser?.snapshotDefaults; + const format = + input.snapshotFormat === "ai" || input.snapshotFormat === "aria" ? input.snapshotFormat : "ai"; + const mode = + input.mode === "efficient" + ? "efficient" + : format === "ai" && snapshotDefaults?.mode === "efficient" + ? "efficient" + : undefined; + const labels = typeof input.labels === "boolean" ? input.labels : undefined; + const refs = input.refs === "aria" || input.refs === "role" ? input.refs : undefined; + const hasMaxChars = Object.hasOwn(input, "maxChars"); + const targetId = typeof input.targetId === "string" ? input.targetId.trim() : undefined; + const limit = + typeof input.limit === "number" && Number.isFinite(input.limit) ? input.limit : undefined; + const maxChars = + typeof input.maxChars === "number" && Number.isFinite(input.maxChars) && input.maxChars > 0 + ? Math.floor(input.maxChars) + : undefined; + const resolvedMaxChars = + format === "ai" + ? hasMaxChars + ? maxChars + : mode === "efficient" + ? undefined + : DEFAULT_AI_SNAPSHOT_MAX_CHARS + : undefined; + const interactive = typeof input.interactive === "boolean" ? input.interactive : undefined; + const compact = typeof input.compact === "boolean" ? input.compact : undefined; + const depth = + typeof input.depth === "number" && Number.isFinite(input.depth) ? input.depth : undefined; + const selector = typeof input.selector === "string" ? input.selector.trim() : undefined; + const frame = typeof input.frame === "string" ? input.frame.trim() : undefined; + const snapshot = proxyRequest + ? ((await proxyRequest({ + method: "GET", + path: "/snapshot", + profile, + query: { + format, + targetId, + limit, + ...(typeof resolvedMaxChars === "number" ? { maxChars: resolvedMaxChars } : {}), + refs, + interactive, + compact, + depth, + selector, + frame, + labels, + mode, + }, + })) as Awaited>) + : await browserSnapshot(baseUrl, { + format, + targetId, + limit, + ...(typeof resolvedMaxChars === "number" ? { maxChars: resolvedMaxChars } : {}), + refs, + interactive, + compact, + depth, + selector, + frame, + labels, + mode, + profile, + }); + if (snapshot.format === "ai") { + const extractedText = snapshot.snapshot ?? ""; + const wrappedSnapshot = wrapExternalContent(extractedText, { + source: "browser", + includeWarning: true, + }); + const safeDetails = { + ok: true, + format: snapshot.format, + targetId: snapshot.targetId, + url: snapshot.url, + truncated: snapshot.truncated, + stats: snapshot.stats, + refs: snapshot.refs ? Object.keys(snapshot.refs).length : undefined, + labels: snapshot.labels, + labelsCount: snapshot.labelsCount, + labelsSkipped: snapshot.labelsSkipped, + imagePath: snapshot.imagePath, + imageType: snapshot.imageType, + externalContent: { + untrusted: true, + source: "browser", + kind: "snapshot", + format: "ai", + wrapped: true, + }, + }; + if (labels && snapshot.imagePath) { + return await imageResultFromFile({ + label: "browser:snapshot", + path: snapshot.imagePath, + extraText: wrappedSnapshot, + details: safeDetails, + }); + } + return { + content: [{ type: "text" as const, text: wrappedSnapshot }], + details: safeDetails, + }; + } + { + const wrapped = wrapBrowserExternalJson({ + kind: "snapshot", + payload: snapshot, + }); + return { + content: [{ type: "text" as const, text: wrapped.wrappedText }], + details: { + ...wrapped.safeDetails, + format: "aria", + targetId: snapshot.targetId, + url: snapshot.url, + nodeCount: snapshot.nodes.length, + externalContent: { + untrusted: true, + source: "browser", + kind: "snapshot", + format: "aria", + wrapped: true, + }, + }, + }; + } +} + +export async function executeConsoleAction(params: { + input: Record; + baseUrl?: string; + profile?: string; + proxyRequest: BrowserProxyRequest | null; +}): Promise> { + const { input, baseUrl, profile, proxyRequest } = params; + const level = typeof input.level === "string" ? input.level.trim() : undefined; + const targetId = typeof input.targetId === "string" ? input.targetId.trim() : undefined; + if (proxyRequest) { + const result = (await proxyRequest({ + method: "GET", + path: "/console", + profile, + query: { + level, + targetId, + }, + })) as { ok?: boolean; targetId?: string; messages?: unknown[] }; + const wrapped = wrapBrowserExternalJson({ + kind: "console", + payload: result, + includeWarning: false, + }); + return { + content: [{ type: "text" as const, text: wrapped.wrappedText }], + details: { + ...wrapped.safeDetails, + targetId: typeof result.targetId === "string" ? result.targetId : undefined, + messageCount: Array.isArray(result.messages) ? result.messages.length : undefined, + }, + }; + } + const result = await browserConsoleMessages(baseUrl, { level, targetId, profile }); + const wrapped = wrapBrowserExternalJson({ + kind: "console", + payload: result, + includeWarning: false, + }); + return { + content: [{ type: "text" as const, text: wrapped.wrappedText }], + details: { + ...wrapped.safeDetails, + targetId: result.targetId, + messageCount: result.messages.length, + }, + }; +} + +export async function executeActAction(params: { + request: Parameters[1]; + baseUrl?: string; + profile?: string; + proxyRequest: BrowserProxyRequest | null; +}): Promise> { + const { request, baseUrl, profile, proxyRequest } = params; + try { + const result = proxyRequest + ? await proxyRequest({ + method: "POST", + path: "/act", + profile, + body: request, + }) + : await browserAct(baseUrl, request, { + profile, + }); + return jsonResult(result); + } catch (err) { + if (isChromeStaleTargetError(profile, err)) { + const retryRequest = stripTargetIdFromActRequest(request); + // Some Chrome relay targetIds can go stale between snapshots and actions. + // Retry once without targetId to let relay use the currently attached tab. + if (retryRequest) { + try { + const retryResult = proxyRequest + ? await proxyRequest({ + method: "POST", + path: "/act", + profile, + body: retryRequest, + }) + : await browserAct(baseUrl, retryRequest, { + profile, + }); + return jsonResult(retryResult); + } catch { + // Fall through to explicit stale-target guidance. + } + } + const tabs = proxyRequest + ? (( + (await proxyRequest({ + method: "GET", + path: "/tabs", + profile, + })) as { tabs?: unknown[] } + ).tabs ?? []) + : await browserTabs(baseUrl, { profile }).catch(() => []); + if (!tabs.length) { + throw new Error( + "No Chrome tabs are attached via the OpenClaw Browser Relay extension. Click the toolbar icon on the tab you want to control (badge ON), then retry.", + { cause: err }, + ); + } + throw new Error( + `Chrome tab not found (stale targetId?). Run action=tabs profile="chrome" and use one of the returned targetIds.`, + { cause: err }, + ); + } + throw err; + } +} diff --git a/src/agents/tools/browser-tool.ts b/src/agents/tools/browser-tool.ts index 74f629b2a50..520b21f021c 100644 --- a/src/agents/tools/browser-tool.ts +++ b/src/agents/tools/browser-tool.ts @@ -1,10 +1,8 @@ import crypto from "node:crypto"; -import type { AgentToolResult } from "@mariozechner/pi-agent-core"; import { browserAct, browserArmDialog, browserArmFileChooser, - browserConsoleMessages, browserNavigate, browserPdfSave, browserScreenshotAction, @@ -14,18 +12,20 @@ import { browserFocusTab, browserOpenTab, browserProfiles, - browserSnapshot, browserStart, browserStatus, browserStop, - browserTabs, } from "../../browser/client.js"; import { resolveBrowserConfig } from "../../browser/config.js"; -import { DEFAULT_AI_SNAPSHOT_MAX_CHARS } from "../../browser/constants.js"; import { DEFAULT_UPLOAD_DIR, resolveExistingPathsWithinRoot } from "../../browser/paths.js"; import { applyBrowserProxyPaths, persistBrowserProxyFiles } from "../../browser/proxy-files.js"; import { loadConfig } from "../../config/config.js"; -import { wrapExternalContent } from "../../security/external-content.js"; +import { + executeActAction, + executeConsoleAction, + executeSnapshotAction, + executeTabsAction, +} from "./browser-tool.actions.js"; import { BrowserToolSchema } from "./browser-tool.schema.js"; import { type AnyAgentTool, imageResultFromFile, jsonResult, readStringParam } from "./common.js"; import { callGatewayTool } from "./gateway.js"; @@ -36,45 +36,6 @@ import { type NodeListNode, } from "./nodes-utils.js"; -function wrapBrowserExternalJson(params: { - kind: "snapshot" | "console" | "tabs"; - payload: unknown; - includeWarning?: boolean; -}): { wrappedText: string; safeDetails: Record } { - const extractedText = JSON.stringify(params.payload, null, 2); - const wrappedText = wrapExternalContent(extractedText, { - source: "browser", - includeWarning: params.includeWarning ?? true, - }); - return { - wrappedText, - safeDetails: { - ok: true, - externalContent: { - untrusted: true, - source: "browser", - kind: params.kind, - wrapped: true, - }, - }, - }; -} - -function formatTabsToolResult(tabs: unknown[]): AgentToolResult { - const wrapped = wrapBrowserExternalJson({ - kind: "tabs", - payload: { tabs }, - includeWarning: false, - }); - const content: AgentToolResult["content"] = [ - { type: "text", text: wrapped.wrappedText }, - ]; - return { - content, - details: { ...wrapped.safeDetails, tabCount: tabs.length }, - }; -} - function readOptionalTargetAndTimeout(params: Record) { const targetId = typeof params.targetId === "string" ? params.targetId.trim() : undefined; const timeoutMs = @@ -138,26 +99,6 @@ function readActRequestParam(params: Record) { return request as Parameters[1]; } -function isChromeStaleTargetError(profile: string | undefined, err: unknown): boolean { - if (profile !== "chrome") { - return false; - } - const msg = String(err); - return msg.includes("404:") && msg.includes("tab not found"); -} - -function stripTargetIdFromActRequest( - request: Parameters[1], -): Parameters[1] | null { - const targetId = typeof request.targetId === "string" ? request.targetId.trim() : undefined; - if (!targetId) { - return null; - } - const retryRequest = { ...request }; - delete retryRequest.targetId; - return retryRequest as Parameters[1]; -} - type BrowserProxyFile = { path: string; base64: string; @@ -465,19 +406,7 @@ export function createBrowserTool(opts?: { } return jsonResult({ profiles: await browserProfiles(baseUrl) }); case "tabs": - if (proxyRequest) { - const result = await proxyRequest({ - method: "GET", - path: "/tabs", - profile, - }); - const tabs = (result as { tabs?: unknown[] }).tabs ?? []; - return formatTabsToolResult(tabs); - } - { - const tabs = await browserTabs(baseUrl, { profile }); - return formatTabsToolResult(tabs); - } + return await executeTabsAction({ baseUrl, profile, proxyRequest }); case "open": { const targetUrl = readTargetUrlParam(params); if (proxyRequest) { @@ -531,148 +460,13 @@ export function createBrowserTool(opts?: { } return jsonResult({ ok: true }); } - case "snapshot": { - const snapshotDefaults = loadConfig().browser?.snapshotDefaults; - const format = - params.snapshotFormat === "ai" || params.snapshotFormat === "aria" - ? params.snapshotFormat - : "ai"; - const mode = - params.mode === "efficient" - ? "efficient" - : format === "ai" && snapshotDefaults?.mode === "efficient" - ? "efficient" - : undefined; - const labels = typeof params.labels === "boolean" ? params.labels : undefined; - const refs = params.refs === "aria" || params.refs === "role" ? params.refs : undefined; - const hasMaxChars = Object.hasOwn(params, "maxChars"); - const targetId = typeof params.targetId === "string" ? params.targetId.trim() : undefined; - const limit = - typeof params.limit === "number" && Number.isFinite(params.limit) - ? params.limit - : undefined; - const maxChars = - typeof params.maxChars === "number" && - Number.isFinite(params.maxChars) && - params.maxChars > 0 - ? Math.floor(params.maxChars) - : undefined; - const resolvedMaxChars = - format === "ai" - ? hasMaxChars - ? maxChars - : mode === "efficient" - ? undefined - : DEFAULT_AI_SNAPSHOT_MAX_CHARS - : undefined; - const interactive = - typeof params.interactive === "boolean" ? params.interactive : undefined; - const compact = typeof params.compact === "boolean" ? params.compact : undefined; - const depth = - typeof params.depth === "number" && Number.isFinite(params.depth) - ? params.depth - : undefined; - const selector = typeof params.selector === "string" ? params.selector.trim() : undefined; - const frame = typeof params.frame === "string" ? params.frame.trim() : undefined; - const snapshot = proxyRequest - ? ((await proxyRequest({ - method: "GET", - path: "/snapshot", - profile, - query: { - format, - targetId, - limit, - ...(typeof resolvedMaxChars === "number" ? { maxChars: resolvedMaxChars } : {}), - refs, - interactive, - compact, - depth, - selector, - frame, - labels, - mode, - }, - })) as Awaited>) - : await browserSnapshot(baseUrl, { - format, - targetId, - limit, - ...(typeof resolvedMaxChars === "number" ? { maxChars: resolvedMaxChars } : {}), - refs, - interactive, - compact, - depth, - selector, - frame, - labels, - mode, - profile, - }); - if (snapshot.format === "ai") { - const extractedText = snapshot.snapshot ?? ""; - const wrappedSnapshot = wrapExternalContent(extractedText, { - source: "browser", - includeWarning: true, - }); - const safeDetails = { - ok: true, - format: snapshot.format, - targetId: snapshot.targetId, - url: snapshot.url, - truncated: snapshot.truncated, - stats: snapshot.stats, - refs: snapshot.refs ? Object.keys(snapshot.refs).length : undefined, - labels: snapshot.labels, - labelsCount: snapshot.labelsCount, - labelsSkipped: snapshot.labelsSkipped, - imagePath: snapshot.imagePath, - imageType: snapshot.imageType, - externalContent: { - untrusted: true, - source: "browser", - kind: "snapshot", - format: "ai", - wrapped: true, - }, - }; - if (labels && snapshot.imagePath) { - return await imageResultFromFile({ - label: "browser:snapshot", - path: snapshot.imagePath, - extraText: wrappedSnapshot, - details: safeDetails, - }); - } - return { - content: [{ type: "text" as const, text: wrappedSnapshot }], - details: safeDetails, - }; - } - { - const wrapped = wrapBrowserExternalJson({ - kind: "snapshot", - payload: snapshot, - }); - return { - content: [{ type: "text" as const, text: wrapped.wrappedText }], - details: { - ...wrapped.safeDetails, - format: "aria", - targetId: snapshot.targetId, - url: snapshot.url, - nodeCount: snapshot.nodes.length, - externalContent: { - untrusted: true, - source: "browser", - kind: "snapshot", - format: "aria", - wrapped: true, - }, - }, - }; - } - } + case "snapshot": + return await executeSnapshotAction({ + input: params, + baseUrl, + profile, + proxyRequest, + }); case "screenshot": { const targetId = readStringParam(params, "targetId"); const fullPage = Boolean(params.fullPage); @@ -729,50 +523,13 @@ export function createBrowserTool(opts?: { }), ); } - case "console": { - const level = typeof params.level === "string" ? params.level.trim() : undefined; - const targetId = typeof params.targetId === "string" ? params.targetId.trim() : undefined; - if (proxyRequest) { - const result = (await proxyRequest({ - method: "GET", - path: "/console", - profile, - query: { - level, - targetId, - }, - })) as { ok?: boolean; targetId?: string; messages?: unknown[] }; - const wrapped = wrapBrowserExternalJson({ - kind: "console", - payload: result, - includeWarning: false, - }); - return { - content: [{ type: "text" as const, text: wrapped.wrappedText }], - details: { - ...wrapped.safeDetails, - targetId: typeof result.targetId === "string" ? result.targetId : undefined, - messageCount: Array.isArray(result.messages) ? result.messages.length : undefined, - }, - }; - } - { - const result = await browserConsoleMessages(baseUrl, { level, targetId, profile }); - const wrapped = wrapBrowserExternalJson({ - kind: "console", - payload: result, - includeWarning: false, - }); - return { - content: [{ type: "text" as const, text: wrapped.wrappedText }], - details: { - ...wrapped.safeDetails, - targetId: result.targetId, - messageCount: result.messages.length, - }, - }; - } - } + case "console": + return await executeConsoleAction({ + input: params, + baseUrl, + profile, + proxyRequest, + }); case "pdf": { const targetId = typeof params.targetId === "string" ? params.targetId.trim() : undefined; const result = proxyRequest @@ -867,62 +624,12 @@ export function createBrowserTool(opts?: { if (!request) { throw new Error("request required"); } - try { - const result = proxyRequest - ? await proxyRequest({ - method: "POST", - path: "/act", - profile, - body: request, - }) - : await browserAct(baseUrl, request, { - profile, - }); - return jsonResult(result); - } catch (err) { - if (isChromeStaleTargetError(profile, err)) { - const retryRequest = stripTargetIdFromActRequest(request); - // Some Chrome relay targetIds can go stale between snapshots and actions. - // Retry once without targetId to let relay use the currently attached tab. - if (retryRequest) { - try { - const retryResult = proxyRequest - ? await proxyRequest({ - method: "POST", - path: "/act", - profile, - body: retryRequest, - }) - : await browserAct(baseUrl, retryRequest, { - profile, - }); - return jsonResult(retryResult); - } catch { - // Fall through to explicit stale-target guidance. - } - } - const tabs = proxyRequest - ? (( - (await proxyRequest({ - method: "GET", - path: "/tabs", - profile, - })) as { tabs?: unknown[] } - ).tabs ?? []) - : await browserTabs(baseUrl, { profile }).catch(() => []); - if (!tabs.length) { - throw new Error( - "No Chrome tabs are attached via the OpenClaw Browser Relay extension. Click the toolbar icon on the tab you want to control (badge ON), then retry.", - { cause: err }, - ); - } - throw new Error( - `Chrome tab not found (stale targetId?). Run action=tabs profile="chrome" and use one of the returned targetIds.`, - { cause: err }, - ); - } - throw err; - } + return await executeActAction({ + request, + baseUrl, + profile, + proxyRequest, + }); } default: throw new Error(`Unknown action: ${action}`); diff --git a/src/browser/cdp-proxy-bypass.ts b/src/browser/cdp-proxy-bypass.ts index 6c607ab8ef2..7f579370de2 100644 --- a/src/browser/cdp-proxy-bypass.ts +++ b/src/browser/cdp-proxy-bypass.ts @@ -50,21 +50,7 @@ export function hasProxyEnv(): boolean { ); } -/** - * Reentrant-safe NO_PROXY extension for CDP localhost connections. - * - * Uses a reference counter so concurrent async callers share the same - * env-var mutation window. The env vars are set on first entry and - * restored on last exit, avoiding the snapshot/restore race that would - * permanently leak NO_PROXY when calls overlap. - */ -let noProxyRefCount = 0; -let savedNoProxy: string | undefined; -let savedNoProxyLower: string | undefined; -let appliedNoProxy: string | undefined; - const LOOPBACK_ENTRIES = "localhost,127.0.0.1,[::1]"; -let noProxyDidModify = false; function noProxyAlreadyCoversLocalhost(): boolean { const current = process.env.NO_PROXY || process.env.no_proxy || ""; @@ -85,6 +71,76 @@ function isLoopbackCdpUrl(url: string): boolean { } } +type NoProxySnapshot = { + noProxy: string | undefined; + noProxyLower: string | undefined; + applied: string; +}; + +class NoProxyLeaseManager { + private leaseCount = 0; + private snapshot: NoProxySnapshot | null = null; + + acquire(url: string): (() => void) | null { + if (!isLoopbackCdpUrl(url) || !hasProxyEnv()) { + return null; + } + + if (this.leaseCount === 0 && !noProxyAlreadyCoversLocalhost()) { + const noProxy = process.env.NO_PROXY; + const noProxyLower = process.env.no_proxy; + const current = noProxy || noProxyLower || ""; + const applied = current ? `${current},${LOOPBACK_ENTRIES}` : LOOPBACK_ENTRIES; + process.env.NO_PROXY = applied; + process.env.no_proxy = applied; + this.snapshot = { noProxy, noProxyLower, applied }; + } + + this.leaseCount += 1; + let released = false; + return () => { + if (released) { + return; + } + released = true; + this.release(); + }; + } + + private release() { + if (this.leaseCount <= 0) { + return; + } + this.leaseCount -= 1; + if (this.leaseCount > 0 || !this.snapshot) { + return; + } + + const { noProxy, noProxyLower, applied } = this.snapshot; + const currentNoProxy = process.env.NO_PROXY; + const currentNoProxyLower = process.env.no_proxy; + const untouched = + currentNoProxy === applied && + (currentNoProxyLower === applied || currentNoProxyLower === undefined); + if (untouched) { + if (noProxy !== undefined) { + process.env.NO_PROXY = noProxy; + } else { + delete process.env.NO_PROXY; + } + if (noProxyLower !== undefined) { + process.env.no_proxy = noProxyLower; + } else { + delete process.env.no_proxy; + } + } + + this.snapshot = null; + } +} + +const noProxyLeaseManager = new NoProxyLeaseManager(); + /** * Scoped NO_PROXY bypass for loopback CDP URLs. * @@ -93,50 +149,10 @@ function isLoopbackCdpUrl(url: string): boolean { * were in-flight. */ export async function withNoProxyForCdpUrl(url: string, fn: () => Promise): Promise { - if (!isLoopbackCdpUrl(url) || !hasProxyEnv()) { - return await fn(); - } - - const isFirst = noProxyRefCount === 0; - noProxyRefCount++; - - if (isFirst && !noProxyAlreadyCoversLocalhost()) { - savedNoProxy = process.env.NO_PROXY; - savedNoProxyLower = process.env.no_proxy; - const current = savedNoProxy || savedNoProxyLower || ""; - const extended = current ? `${current},${LOOPBACK_ENTRIES}` : LOOPBACK_ENTRIES; - process.env.NO_PROXY = extended; - process.env.no_proxy = extended; - appliedNoProxy = extended; - noProxyDidModify = true; - } - + const release = noProxyLeaseManager.acquire(url); try { return await fn(); } finally { - noProxyRefCount--; - if (noProxyRefCount === 0 && noProxyDidModify) { - const currentNoProxy = process.env.NO_PROXY; - const currentNoProxyLower = process.env.no_proxy; - const untouched = - currentNoProxy === appliedNoProxy && - (currentNoProxyLower === appliedNoProxy || currentNoProxyLower === undefined); - if (untouched) { - if (savedNoProxy !== undefined) { - process.env.NO_PROXY = savedNoProxy; - } else { - delete process.env.NO_PROXY; - } - if (savedNoProxyLower !== undefined) { - process.env.no_proxy = savedNoProxyLower; - } else { - delete process.env.no_proxy; - } - } - savedNoProxy = undefined; - savedNoProxyLower = undefined; - appliedNoProxy = undefined; - noProxyDidModify = false; - } + release?.(); } } diff --git a/src/browser/cdp-timeouts.test.ts b/src/browser/cdp-timeouts.test.ts new file mode 100644 index 00000000000..178915dc78a --- /dev/null +++ b/src/browser/cdp-timeouts.test.ts @@ -0,0 +1,69 @@ +import { describe, expect, it } from "vitest"; +import { + PROFILE_HTTP_REACHABILITY_TIMEOUT_MS, + PROFILE_WS_REACHABILITY_MAX_TIMEOUT_MS, + PROFILE_WS_REACHABILITY_MIN_TIMEOUT_MS, + resolveCdpReachabilityTimeouts, +} from "./cdp-timeouts.js"; + +describe("resolveCdpReachabilityTimeouts", () => { + it("uses loopback defaults when timeout is omitted", () => { + expect( + resolveCdpReachabilityTimeouts({ + profileIsLoopback: true, + timeoutMs: undefined, + remoteHttpTimeoutMs: 1500, + remoteHandshakeTimeoutMs: 3000, + }), + ).toEqual({ + httpTimeoutMs: PROFILE_HTTP_REACHABILITY_TIMEOUT_MS, + wsTimeoutMs: PROFILE_HTTP_REACHABILITY_TIMEOUT_MS * 2, + }); + }); + + it("clamps loopback websocket timeout range", () => { + const low = resolveCdpReachabilityTimeouts({ + profileIsLoopback: true, + timeoutMs: 1, + remoteHttpTimeoutMs: 1500, + remoteHandshakeTimeoutMs: 3000, + }); + const high = resolveCdpReachabilityTimeouts({ + profileIsLoopback: true, + timeoutMs: 5000, + remoteHttpTimeoutMs: 1500, + remoteHandshakeTimeoutMs: 3000, + }); + + expect(low.wsTimeoutMs).toBe(PROFILE_WS_REACHABILITY_MIN_TIMEOUT_MS); + expect(high.wsTimeoutMs).toBe(PROFILE_WS_REACHABILITY_MAX_TIMEOUT_MS); + }); + + it("enforces remote minimums even when caller passes lower timeout", () => { + expect( + resolveCdpReachabilityTimeouts({ + profileIsLoopback: false, + timeoutMs: 200, + remoteHttpTimeoutMs: 1500, + remoteHandshakeTimeoutMs: 3000, + }), + ).toEqual({ + httpTimeoutMs: 1500, + wsTimeoutMs: 3000, + }); + }); + + it("uses remote defaults when timeout is omitted", () => { + expect( + resolveCdpReachabilityTimeouts({ + profileIsLoopback: false, + timeoutMs: undefined, + remoteHttpTimeoutMs: 1750, + remoteHandshakeTimeoutMs: 3250, + }), + ).toEqual({ + httpTimeoutMs: 1750, + wsTimeoutMs: 3250, + }); + }); +}); diff --git a/src/browser/cdp-timeouts.ts b/src/browser/cdp-timeouts.ts new file mode 100644 index 00000000000..5641a53cc93 --- /dev/null +++ b/src/browser/cdp-timeouts.ts @@ -0,0 +1,54 @@ +export const CDP_HTTP_REQUEST_TIMEOUT_MS = 1500; +export const CDP_WS_HANDSHAKE_TIMEOUT_MS = 5000; +export const CDP_JSON_NEW_TIMEOUT_MS = 1500; + +export const CHROME_REACHABILITY_TIMEOUT_MS = 500; +export const CHROME_WS_READY_TIMEOUT_MS = 800; +export const CHROME_BOOTSTRAP_PREFS_TIMEOUT_MS = 10_000; +export const CHROME_BOOTSTRAP_EXIT_TIMEOUT_MS = 5000; +export const CHROME_LAUNCH_READY_WINDOW_MS = 15_000; +export const CHROME_LAUNCH_READY_POLL_MS = 200; +export const CHROME_STOP_TIMEOUT_MS = 2500; +export const CHROME_STOP_PROBE_TIMEOUT_MS = 200; +export const CHROME_STDERR_HINT_MAX_CHARS = 2000; + +export const PROFILE_HTTP_REACHABILITY_TIMEOUT_MS = 300; +export const PROFILE_WS_REACHABILITY_MIN_TIMEOUT_MS = 200; +export const PROFILE_WS_REACHABILITY_MAX_TIMEOUT_MS = 2000; +export const PROFILE_ATTACH_RETRY_TIMEOUT_MS = 1200; +export const PROFILE_POST_RESTART_WS_TIMEOUT_MS = 600; + +function normalizeTimeoutMs(value: number | undefined): number | undefined { + if (typeof value !== "number" || !Number.isFinite(value)) { + return undefined; + } + return Math.max(1, Math.floor(value)); +} + +export function resolveCdpReachabilityTimeouts(params: { + profileIsLoopback: boolean; + timeoutMs?: number; + remoteHttpTimeoutMs: number; + remoteHandshakeTimeoutMs: number; +}): { httpTimeoutMs: number; wsTimeoutMs: number } { + const normalized = normalizeTimeoutMs(params.timeoutMs); + if (params.profileIsLoopback) { + const httpTimeoutMs = normalized ?? PROFILE_HTTP_REACHABILITY_TIMEOUT_MS; + const wsTimeoutMs = Math.max( + PROFILE_WS_REACHABILITY_MIN_TIMEOUT_MS, + Math.min(PROFILE_WS_REACHABILITY_MAX_TIMEOUT_MS, httpTimeoutMs * 2), + ); + return { httpTimeoutMs, wsTimeoutMs }; + } + + if (normalized !== undefined) { + return { + httpTimeoutMs: Math.max(normalized, params.remoteHttpTimeoutMs), + wsTimeoutMs: Math.max(normalized * 2, params.remoteHandshakeTimeoutMs), + }; + } + return { + httpTimeoutMs: params.remoteHttpTimeoutMs, + wsTimeoutMs: params.remoteHandshakeTimeoutMs, + }; +} diff --git a/src/browser/cdp.helpers.ts b/src/browser/cdp.helpers.ts index 15276100f92..0ae9d22d80b 100644 --- a/src/browser/cdp.helpers.ts +++ b/src/browser/cdp.helpers.ts @@ -2,6 +2,7 @@ import WebSocket from "ws"; import { isLoopbackHost } from "../gateway/net.js"; import { rawDataToString } from "../infra/ws.js"; import { getDirectAgentForCdp, withNoProxyForCdpUrl } from "./cdp-proxy-bypass.js"; +import { CDP_HTTP_REQUEST_TIMEOUT_MS, CDP_WS_HANDSHAKE_TIMEOUT_MS } from "./cdp-timeouts.js"; import { getChromeExtensionRelayAuthHeaders } from "./extension-relay.js"; export { isLoopbackHost }; @@ -113,14 +114,18 @@ function createCdpSender(ws: WebSocket) { return { send, closeWithError }; } -export async function fetchJson(url: string, timeoutMs = 1500, init?: RequestInit): Promise { +export async function fetchJson( + url: string, + timeoutMs = CDP_HTTP_REQUEST_TIMEOUT_MS, + init?: RequestInit, +): Promise { const res = await fetchCdpChecked(url, timeoutMs, init); return (await res.json()) as T; } export async function fetchCdpChecked( url: string, - timeoutMs = 1500, + timeoutMs = CDP_HTTP_REQUEST_TIMEOUT_MS, init?: RequestInit, ): Promise { const ctrl = new AbortController(); @@ -139,7 +144,11 @@ export async function fetchCdpChecked( } } -export async function fetchOk(url: string, timeoutMs = 1500, init?: RequestInit): Promise { +export async function fetchOk( + url: string, + timeoutMs = CDP_HTTP_REQUEST_TIMEOUT_MS, + init?: RequestInit, +): Promise { await fetchCdpChecked(url, timeoutMs, init); } @@ -151,7 +160,7 @@ export function openCdpWebSocket( const handshakeTimeoutMs = typeof opts?.handshakeTimeoutMs === "number" && Number.isFinite(opts.handshakeTimeoutMs) ? Math.max(1, Math.floor(opts.handshakeTimeoutMs)) - : 5000; + : CDP_WS_HANDSHAKE_TIMEOUT_MS; const agent = getDirectAgentForCdp(wsUrl); return new WebSocket(wsUrl, { handshakeTimeout: handshakeTimeoutMs, diff --git a/src/browser/chrome.ts b/src/browser/chrome.ts index 5d51b6e59bd..ab21fd6f0a0 100644 --- a/src/browser/chrome.ts +++ b/src/browser/chrome.ts @@ -5,6 +5,17 @@ import path from "node:path"; import { ensurePortAvailable } from "../infra/ports.js"; import { createSubsystemLogger } from "../logging/subsystem.js"; import { CONFIG_DIR } from "../utils.js"; +import { + CHROME_BOOTSTRAP_EXIT_TIMEOUT_MS, + CHROME_BOOTSTRAP_PREFS_TIMEOUT_MS, + CHROME_LAUNCH_READY_POLL_MS, + CHROME_LAUNCH_READY_WINDOW_MS, + CHROME_REACHABILITY_TIMEOUT_MS, + CHROME_STDERR_HINT_MAX_CHARS, + CHROME_STOP_PROBE_TIMEOUT_MS, + CHROME_STOP_TIMEOUT_MS, + CHROME_WS_READY_TIMEOUT_MS, +} from "./cdp-timeouts.js"; import { appendCdpPath, fetchCdpChecked, openCdpWebSocket } from "./cdp.helpers.js"; import { normalizeCdpWsUrl } from "./cdp.js"; import { @@ -66,7 +77,10 @@ function cdpUrlForPort(cdpPort: number) { return `http://127.0.0.1:${cdpPort}`; } -export async function isChromeReachable(cdpUrl: string, timeoutMs = 500): Promise { +export async function isChromeReachable( + cdpUrl: string, + timeoutMs = CHROME_REACHABILITY_TIMEOUT_MS, +): Promise { const version = await fetchChromeVersion(cdpUrl, timeoutMs); return Boolean(version); } @@ -77,7 +91,10 @@ type ChromeVersion = { "User-Agent"?: string; }; -async function fetchChromeVersion(cdpUrl: string, timeoutMs = 500): Promise { +async function fetchChromeVersion( + cdpUrl: string, + timeoutMs = CHROME_REACHABILITY_TIMEOUT_MS, +): Promise { const ctrl = new AbortController(); const t = setTimeout(ctrl.abort.bind(ctrl), timeoutMs); try { @@ -97,7 +114,7 @@ async function fetchChromeVersion(cdpUrl: string, timeoutMs = 500): Promise { const version = await fetchChromeVersion(cdpUrl, timeoutMs); const wsUrl = String(version?.webSocketDebuggerUrl ?? "").trim(); @@ -107,7 +124,10 @@ export async function getChromeWebSocketUrl( return normalizeCdpWsUrl(wsUrl, cdpUrl); } -async function canOpenWebSocket(wsUrl: string, timeoutMs = 800): Promise { +async function canOpenWebSocket( + wsUrl: string, + timeoutMs = CHROME_WS_READY_TIMEOUT_MS, +): Promise { return await new Promise((resolve) => { const ws = openCdpWebSocket(wsUrl, { handshakeTimeoutMs: timeoutMs, @@ -141,8 +161,8 @@ async function canOpenWebSocket(wsUrl: string, timeoutMs = 800): Promise { const wsUrl = await getChromeWebSocketUrl(cdpUrl, timeoutMs); if (!wsUrl) { @@ -236,7 +256,7 @@ export async function launchOpenClawChrome( // Then decorate (if needed) before the "real" run. if (needsBootstrap) { const bootstrap = spawnOnce(); - const deadline = Date.now() + 10_000; + const deadline = Date.now() + CHROME_BOOTSTRAP_PREFS_TIMEOUT_MS; while (Date.now() < deadline) { if (exists(localStatePath) && exists(preferencesPath)) { break; @@ -248,7 +268,7 @@ export async function launchOpenClawChrome( } catch { // ignore } - const exitDeadline = Date.now() + 5000; + const exitDeadline = Date.now() + CHROME_BOOTSTRAP_EXIT_TIMEOUT_MS; while (Date.now() < exitDeadline) { if (bootstrap.exitCode != null) { break; @@ -287,17 +307,19 @@ export async function launchOpenClawChrome( proc.stderr?.on("data", onStderr); // Wait for CDP to come up. - const readyDeadline = Date.now() + 15_000; + const readyDeadline = Date.now() + CHROME_LAUNCH_READY_WINDOW_MS; while (Date.now() < readyDeadline) { - if (await isChromeReachable(profile.cdpUrl, 500)) { + if (await isChromeReachable(profile.cdpUrl)) { break; } - await new Promise((r) => setTimeout(r, 200)); + await new Promise((r) => setTimeout(r, CHROME_LAUNCH_READY_POLL_MS)); } - if (!(await isChromeReachable(profile.cdpUrl, 500))) { + if (!(await isChromeReachable(profile.cdpUrl))) { const stderrOutput = Buffer.concat(stderrChunks).toString("utf8").trim(); - const stderrHint = stderrOutput ? `\nChrome stderr:\n${stderrOutput.slice(0, 2000)}` : ""; + const stderrHint = stderrOutput + ? `\nChrome stderr:\n${stderrOutput.slice(0, CHROME_STDERR_HINT_MAX_CHARS)}` + : ""; const sandboxHint = process.platform === "linux" && !resolved.noSandbox ? "\nHint: If running in a container or as root, try setting browser.noSandbox: true in config." @@ -331,7 +353,10 @@ export async function launchOpenClawChrome( }; } -export async function stopOpenClawChrome(running: RunningChrome, timeoutMs = 2500) { +export async function stopOpenClawChrome( + running: RunningChrome, + timeoutMs = CHROME_STOP_TIMEOUT_MS, +) { const proc = running.proc; if (proc.killed) { return; @@ -347,7 +372,7 @@ export async function stopOpenClawChrome(running: RunningChrome, timeoutMs = 250 if (!proc.exitCode && proc.killed) { break; } - if (!(await isChromeReachable(cdpUrlForPort(running.cdpPort), 200))) { + if (!(await isChromeReachable(cdpUrlForPort(running.cdpPort), CHROME_STOP_PROBE_TIMEOUT_MS))) { return; } await new Promise((r) => setTimeout(r, 100)); diff --git a/src/browser/server-context.availability.ts b/src/browser/server-context.availability.ts index 42915efb6c0..47865903b96 100644 --- a/src/browser/server-context.availability.ts +++ b/src/browser/server-context.availability.ts @@ -1,3 +1,8 @@ +import { + PROFILE_ATTACH_RETRY_TIMEOUT_MS, + PROFILE_POST_RESTART_WS_TIMEOUT_MS, + resolveCdpReachabilityTimeouts, +} from "./cdp-timeouts.js"; import { isChromeCdpReady, isChromeReachable, @@ -43,38 +48,22 @@ export function createProfileAvailability({ getProfileState, setProfileRunning, }: AvailabilityDeps): AvailabilityOps { - const resolveRemoteHttpTimeout = (timeoutMs: number | undefined) => { - if (profile.cdpIsLoopback) { - return timeoutMs ?? 300; - } - const resolved = state().resolved; - if (typeof timeoutMs === "number" && Number.isFinite(timeoutMs)) { - return Math.max(Math.floor(timeoutMs), resolved.remoteCdpTimeoutMs); - } - return resolved.remoteCdpTimeoutMs; - }; - - const resolveRemoteWsTimeout = (timeoutMs: number | undefined) => { - if (profile.cdpIsLoopback) { - const base = timeoutMs ?? 300; - return Math.max(200, Math.min(2000, base * 2)); - } - const resolved = state().resolved; - if (typeof timeoutMs === "number" && Number.isFinite(timeoutMs)) { - return Math.max(Math.floor(timeoutMs) * 2, resolved.remoteCdpHandshakeTimeoutMs); - } - return resolved.remoteCdpHandshakeTimeoutMs; - }; + const resolveTimeouts = (timeoutMs: number | undefined) => + resolveCdpReachabilityTimeouts({ + profileIsLoopback: profile.cdpIsLoopback, + timeoutMs, + remoteHttpTimeoutMs: state().resolved.remoteCdpTimeoutMs, + remoteHandshakeTimeoutMs: state().resolved.remoteCdpHandshakeTimeoutMs, + }); const isReachable = async (timeoutMs?: number) => { - const httpTimeout = resolveRemoteHttpTimeout(timeoutMs); - const wsTimeout = resolveRemoteWsTimeout(timeoutMs); - return await isChromeCdpReady(profile.cdpUrl, httpTimeout, wsTimeout); + const { httpTimeoutMs, wsTimeoutMs } = resolveTimeouts(timeoutMs); + return await isChromeCdpReady(profile.cdpUrl, httpTimeoutMs, wsTimeoutMs); }; const isHttpReachable = async (timeoutMs?: number) => { - const httpTimeout = resolveRemoteHttpTimeout(timeoutMs); - return await isChromeReachable(profile.cdpUrl, httpTimeout); + const { httpTimeoutMs } = resolveTimeouts(timeoutMs); + return await isChromeReachable(profile.cdpUrl, httpTimeoutMs); }; const attachRunning = (running: NonNullable) => { @@ -129,7 +118,7 @@ export function createProfileAvailability({ if (isExtension) { if (!httpReachable) { await ensureChromeExtensionRelayServer({ cdpUrl: profile.cdpUrl }); - if (!(await isHttpReachable(1200))) { + if (!(await isHttpReachable(PROFILE_ATTACH_RETRY_TIMEOUT_MS))) { throw new Error( `Chrome extension relay for profile "${profile.name}" is not reachable at ${profile.cdpUrl}.`, ); @@ -143,7 +132,7 @@ export function createProfileAvailability({ if (!httpReachable) { if ((attachOnly || remoteCdp) && opts.onEnsureAttachTarget) { await opts.onEnsureAttachTarget(profile); - if (await isHttpReachable(1200)) { + if (await isHttpReachable(PROFILE_ATTACH_RETRY_TIMEOUT_MS)) { return; } } @@ -176,7 +165,7 @@ export function createProfileAvailability({ if (attachOnly || remoteCdp) { if (opts.onEnsureAttachTarget) { await opts.onEnsureAttachTarget(profile); - if (await isReachable(1200)) { + if (await isReachable(PROFILE_ATTACH_RETRY_TIMEOUT_MS)) { return; } } @@ -201,7 +190,7 @@ export function createProfileAvailability({ const relaunched = await launchOpenClawChrome(current.resolved, profile); attachRunning(relaunched); - if (!(await isReachable(600))) { + if (!(await isReachable(PROFILE_POST_RESTART_WS_TIMEOUT_MS))) { throw new Error( `Chrome CDP websocket for profile "${profile.name}" is not reachable after restart.`, ); diff --git a/src/browser/server-context.remote-profile-tab-ops.test.ts b/src/browser/server-context.remote-profile-tab-ops.test.ts new file mode 100644 index 00000000000..746a8c87f53 --- /dev/null +++ b/src/browser/server-context.remote-profile-tab-ops.test.ts @@ -0,0 +1,273 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import "./server-context.chrome-test-harness.js"; +import * as chromeModule from "./chrome.js"; +import * as pwAiModule from "./pw-ai-module.js"; +import { createBrowserRouteContext } from "./server-context.js"; +import { + createJsonListFetchMock, + createRemoteRouteHarness, + createSequentialPageLister, + makeState, + originalFetch, +} from "./server-context.remote-tab-ops.harness.js"; + +afterEach(() => { + globalThis.fetch = originalFetch; + vi.restoreAllMocks(); +}); + +describe("browser server-context remote profile tab operations", () => { + it("uses profile-level attachOnly when global attachOnly is false", async () => { + const state = makeState("openclaw"); + state.resolved.attachOnly = false; + state.resolved.profiles.openclaw = { + cdpPort: 18800, + attachOnly: true, + color: "#FF4500", + }; + + const reachableMock = vi.mocked(chromeModule.isChromeReachable).mockResolvedValueOnce(false); + const launchMock = vi.mocked(chromeModule.launchOpenClawChrome); + const ctx = createBrowserRouteContext({ getState: () => state }); + + await expect(ctx.forProfile("openclaw").ensureBrowserAvailable()).rejects.toThrow( + /attachOnly is enabled/i, + ); + expect(reachableMock).toHaveBeenCalled(); + expect(launchMock).not.toHaveBeenCalled(); + }); + + it("keeps attachOnly websocket failures off the loopback ownership error path", async () => { + const state = makeState("openclaw"); + state.resolved.attachOnly = false; + state.resolved.profiles.openclaw = { + cdpPort: 18800, + attachOnly: true, + color: "#FF4500", + }; + + const httpReachableMock = vi.mocked(chromeModule.isChromeReachable).mockResolvedValueOnce(true); + const wsReachableMock = vi.mocked(chromeModule.isChromeCdpReady).mockResolvedValueOnce(false); + const launchMock = vi.mocked(chromeModule.launchOpenClawChrome); + const ctx = createBrowserRouteContext({ getState: () => state }); + + await expect(ctx.forProfile("openclaw").ensureBrowserAvailable()).rejects.toThrow( + /attachOnly is enabled and CDP websocket/i, + ); + expect(httpReachableMock).toHaveBeenCalled(); + expect(wsReachableMock).toHaveBeenCalled(); + expect(launchMock).not.toHaveBeenCalled(); + }); + + it("uses Playwright tab operations when available", async () => { + const listPagesViaPlaywright = vi.fn(async () => [ + { targetId: "T1", title: "Tab 1", url: "https://example.com", type: "page" }, + ]); + const createPageViaPlaywright = vi.fn(async () => ({ + targetId: "T2", + title: "Tab 2", + url: "http://127.0.0.1:3000", + type: "page", + })); + const closePageByTargetIdViaPlaywright = vi.fn(async () => {}); + + vi.spyOn(pwAiModule, "getPwAiModule").mockResolvedValue({ + listPagesViaPlaywright, + createPageViaPlaywright, + closePageByTargetIdViaPlaywright, + } as unknown as Awaited>); + + const { state, remote, fetchMock } = createRemoteRouteHarness(); + + const tabs = await remote.listTabs(); + expect(tabs.map((t) => t.targetId)).toEqual(["T1"]); + + const opened = await remote.openTab("http://127.0.0.1:3000"); + expect(opened.targetId).toBe("T2"); + expect(state.profiles.get("remote")?.lastTargetId).toBe("T2"); + expect(createPageViaPlaywright).toHaveBeenCalledWith({ + cdpUrl: "https://browserless.example/chrome?token=abc", + url: "http://127.0.0.1:3000", + ssrfPolicy: { allowPrivateNetwork: true }, + }); + + await remote.closeTab("T1"); + expect(closePageByTargetIdViaPlaywright).toHaveBeenCalledWith({ + cdpUrl: "https://browserless.example/chrome?token=abc", + targetId: "T1", + }); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("prefers lastTargetId for remote profiles when targetId is omitted", async () => { + const responses = [ + [ + { targetId: "A", title: "A", url: "https://example.com", type: "page" }, + { targetId: "B", title: "B", url: "https://www.example.com", type: "page" }, + ], + [ + { targetId: "A", title: "A", url: "https://example.com", type: "page" }, + { targetId: "B", title: "B", url: "https://www.example.com", type: "page" }, + ], + [ + { targetId: "B", title: "B", url: "https://www.example.com", type: "page" }, + { targetId: "A", title: "A", url: "https://example.com", type: "page" }, + ], + [ + { targetId: "B", title: "B", url: "https://www.example.com", type: "page" }, + { targetId: "A", title: "A", url: "https://example.com", type: "page" }, + ], + ]; + + const listPagesViaPlaywright = vi.fn(createSequentialPageLister(responses)); + + vi.spyOn(pwAiModule, "getPwAiModule").mockResolvedValue({ + listPagesViaPlaywright, + createPageViaPlaywright: vi.fn(async () => { + throw new Error("unexpected create"); + }), + closePageByTargetIdViaPlaywright: vi.fn(async () => { + throw new Error("unexpected close"); + }), + } as unknown as Awaited>); + + const { remote } = createRemoteRouteHarness(); + + const first = await remote.ensureTabAvailable(); + expect(first.targetId).toBe("A"); + const second = await remote.ensureTabAvailable(); + expect(second.targetId).toBe("A"); + }); + + it("falls back to the only tab for remote profiles when targetId is stale", async () => { + const responses = [ + [{ targetId: "T1", title: "Tab 1", url: "https://example.com", type: "page" }], + [{ targetId: "T1", title: "Tab 1", url: "https://example.com", type: "page" }], + ]; + const listPagesViaPlaywright = vi.fn(createSequentialPageLister(responses)); + + vi.spyOn(pwAiModule, "getPwAiModule").mockResolvedValue({ + listPagesViaPlaywright, + } as unknown as Awaited>); + + const { remote } = createRemoteRouteHarness(); + const chosen = await remote.ensureTabAvailable("STALE_TARGET"); + expect(chosen.targetId).toBe("T1"); + }); + + it("keeps rejecting stale targetId for remote profiles when multiple tabs exist", async () => { + const responses = [ + [ + { targetId: "A", title: "A", url: "https://a.example", type: "page" }, + { targetId: "B", title: "B", url: "https://b.example", type: "page" }, + ], + [ + { targetId: "A", title: "A", url: "https://a.example", type: "page" }, + { targetId: "B", title: "B", url: "https://b.example", type: "page" }, + ], + ]; + const listPagesViaPlaywright = vi.fn(createSequentialPageLister(responses)); + + vi.spyOn(pwAiModule, "getPwAiModule").mockResolvedValue({ + listPagesViaPlaywright, + } as unknown as Awaited>); + + const { remote } = createRemoteRouteHarness(); + await expect(remote.ensureTabAvailable("STALE_TARGET")).rejects.toThrow(/tab not found/i); + }); + + it("uses Playwright focus for remote profiles when available", async () => { + const listPagesViaPlaywright = vi.fn(async () => [ + { targetId: "T1", title: "Tab 1", url: "https://example.com", type: "page" }, + ]); + const focusPageByTargetIdViaPlaywright = vi.fn(async () => {}); + + vi.spyOn(pwAiModule, "getPwAiModule").mockResolvedValue({ + listPagesViaPlaywright, + focusPageByTargetIdViaPlaywright, + } as unknown as Awaited>); + + const { state, remote, fetchMock } = createRemoteRouteHarness(); + + await remote.focusTab("T1"); + expect(focusPageByTargetIdViaPlaywright).toHaveBeenCalledWith({ + cdpUrl: "https://browserless.example/chrome?token=abc", + targetId: "T1", + }); + expect(fetchMock).not.toHaveBeenCalled(); + expect(state.profiles.get("remote")?.lastTargetId).toBe("T1"); + }); + + it("does not swallow Playwright runtime errors for remote profiles", async () => { + vi.spyOn(pwAiModule, "getPwAiModule").mockResolvedValue({ + listPagesViaPlaywright: vi.fn(async () => { + throw new Error("boom"); + }), + } as unknown as Awaited>); + + const { remote, fetchMock } = createRemoteRouteHarness(); + + await expect(remote.listTabs()).rejects.toThrow(/boom/); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("falls back to /json/list when Playwright is not available", async () => { + vi.spyOn(pwAiModule, "getPwAiModule").mockResolvedValue(null); + const { remote } = createRemoteRouteHarness( + vi.fn( + createJsonListFetchMock([ + { + id: "T1", + title: "Tab 1", + url: "https://example.com", + webSocketDebuggerUrl: "wss://browserless.example/devtools/page/T1", + type: "page", + }, + ]), + ), + ); + + const tabs = await remote.listTabs(); + expect(tabs.map((t) => t.targetId)).toEqual(["T1"]); + }); + + it("does not enforce managed tab cap for remote openclaw profiles", async () => { + const listPagesViaPlaywright = vi + .fn() + .mockResolvedValueOnce([ + { targetId: "T1", title: "1", url: "https://1.example", type: "page" }, + ]) + .mockResolvedValueOnce([ + { targetId: "T1", title: "1", url: "https://1.example", type: "page" }, + { targetId: "T2", title: "2", url: "https://2.example", type: "page" }, + { targetId: "T3", title: "3", url: "https://3.example", type: "page" }, + { targetId: "T4", title: "4", url: "https://4.example", type: "page" }, + { targetId: "T5", title: "5", url: "https://5.example", type: "page" }, + { targetId: "T6", title: "6", url: "https://6.example", type: "page" }, + { targetId: "T7", title: "7", url: "https://7.example", type: "page" }, + { targetId: "T8", title: "8", url: "https://8.example", type: "page" }, + { targetId: "T9", title: "9", url: "https://9.example", type: "page" }, + ]); + + const createPageViaPlaywright = vi.fn(async () => ({ + targetId: "T1", + title: "Tab 1", + url: "https://1.example", + type: "page", + })); + + vi.spyOn(pwAiModule, "getPwAiModule").mockResolvedValue({ + listPagesViaPlaywright, + createPageViaPlaywright, + } as unknown as Awaited>); + + const fetchMock = vi.fn(async (url: unknown) => { + throw new Error(`unexpected fetch: ${String(url)}`); + }); + + const { remote } = createRemoteRouteHarness(fetchMock); + const opened = await remote.openTab("https://1.example"); + expect(opened.targetId).toBe("T1"); + expect(fetchMock).not.toHaveBeenCalled(); + }); +}); diff --git a/src/browser/server-context.remote-tab-ops.harness.ts b/src/browser/server-context.remote-tab-ops.harness.ts new file mode 100644 index 00000000000..c5f65a4ce2a --- /dev/null +++ b/src/browser/server-context.remote-tab-ops.harness.ts @@ -0,0 +1,107 @@ +import { vi } from "vitest"; +import { withFetchPreconnect } from "../test-utils/fetch-mock.js"; +import type { BrowserServerState } from "./server-context.js"; +import { createBrowserRouteContext } from "./server-context.js"; + +export const originalFetch = globalThis.fetch; + +export function makeState( + profile: "remote" | "openclaw", +): BrowserServerState & { profiles: Map } { + return { + // oxlint-disable-next-line typescript/no-explicit-any + server: null as any, + port: 0, + resolved: { + enabled: true, + controlPort: 18791, + cdpPortRangeStart: 18800, + cdpPortRangeEnd: 18899, + cdpProtocol: profile === "remote" ? "https" : "http", + cdpHost: profile === "remote" ? "browserless.example" : "127.0.0.1", + cdpIsLoopback: profile !== "remote", + remoteCdpTimeoutMs: 1500, + remoteCdpHandshakeTimeoutMs: 3000, + evaluateEnabled: false, + extraArgs: [], + color: "#FF4500", + headless: true, + noSandbox: false, + attachOnly: false, + ssrfPolicy: { allowPrivateNetwork: true }, + defaultProfile: profile, + profiles: { + remote: { + cdpUrl: "https://browserless.example/chrome?token=abc", + cdpPort: 443, + color: "#00AA00", + }, + openclaw: { cdpPort: 18800, color: "#FF4500" }, + }, + }, + profiles: new Map(), + }; +} + +export function makeUnexpectedFetchMock() { + return vi.fn(async () => { + throw new Error("unexpected fetch"); + }); +} + +export function createRemoteRouteHarness(fetchMock?: (url: unknown) => Promise) { + const activeFetchMock = fetchMock ?? makeUnexpectedFetchMock(); + global.fetch = withFetchPreconnect(activeFetchMock); + const state = makeState("remote"); + const ctx = createBrowserRouteContext({ getState: () => state }); + return { state, remote: ctx.forProfile("remote"), fetchMock: activeFetchMock }; +} + +export function createSequentialPageLister(responses: T[]) { + return async () => { + const next = responses.shift(); + if (!next) { + throw new Error("no more responses"); + } + return next; + }; +} + +type JsonListEntry = { + id: string; + title: string; + url: string; + webSocketDebuggerUrl: string; + type: "page"; +}; + +export function createJsonListFetchMock(entries: JsonListEntry[]) { + return async (url: unknown) => { + const u = String(url); + if (!u.includes("/json/list")) { + throw new Error(`unexpected fetch: ${u}`); + } + return { + ok: true, + json: async () => entries, + } as unknown as Response; + }; +} + +function makeManagedTab(id: string, ordinal: number): JsonListEntry { + return { + id, + title: String(ordinal), + url: `http://127.0.0.1:300${ordinal}`, + webSocketDebuggerUrl: `ws://127.0.0.1/devtools/page/${id}`, + type: "page", + }; +} + +export function makeManagedTabsWithNew(params?: { newFirst?: boolean }): JsonListEntry[] { + const oldTabs = Array.from({ length: 8 }, (_, index) => + makeManagedTab(`OLD${index + 1}`, index + 1), + ); + const newTab = makeManagedTab("NEW", 9); + return params?.newFirst ? [newTab, ...oldTabs] : [...oldTabs, newTab]; +} diff --git a/src/browser/server-context.remote-tab-ops.test.ts b/src/browser/server-context.remote-tab-ops.test.ts deleted file mode 100644 index ff9afafe1f7..00000000000 --- a/src/browser/server-context.remote-tab-ops.test.ts +++ /dev/null @@ -1,616 +0,0 @@ -import { afterEach, describe, expect, it, vi } from "vitest"; -import { withFetchPreconnect } from "../test-utils/fetch-mock.js"; -import "./server-context.chrome-test-harness.js"; -import * as cdpModule from "./cdp.js"; -import * as chromeModule from "./chrome.js"; -import { InvalidBrowserNavigationUrlError } from "./navigation-guard.js"; -import * as pwAiModule from "./pw-ai-module.js"; -import type { BrowserServerState } from "./server-context.js"; -import { createBrowserRouteContext } from "./server-context.js"; - -const originalFetch = globalThis.fetch; - -afterEach(() => { - globalThis.fetch = originalFetch; - vi.restoreAllMocks(); -}); - -function makeState( - profile: "remote" | "openclaw", -): BrowserServerState & { profiles: Map } { - return { - // oxlint-disable-next-line typescript/no-explicit-any - server: null as any, - port: 0, - resolved: { - enabled: true, - controlPort: 18791, - cdpPortRangeStart: 18800, - cdpPortRangeEnd: 18899, - cdpProtocol: profile === "remote" ? "https" : "http", - cdpHost: profile === "remote" ? "browserless.example" : "127.0.0.1", - cdpIsLoopback: profile !== "remote", - remoteCdpTimeoutMs: 1500, - remoteCdpHandshakeTimeoutMs: 3000, - evaluateEnabled: false, - extraArgs: [], - color: "#FF4500", - headless: true, - noSandbox: false, - attachOnly: false, - ssrfPolicy: { allowPrivateNetwork: true }, - defaultProfile: profile, - profiles: { - remote: { - cdpUrl: "https://browserless.example/chrome?token=abc", - cdpPort: 443, - color: "#00AA00", - }, - openclaw: { cdpPort: 18800, color: "#FF4500" }, - }, - }, - profiles: new Map(), - }; -} - -function makeUnexpectedFetchMock() { - return vi.fn(async () => { - throw new Error("unexpected fetch"); - }); -} - -function createRemoteRouteHarness(fetchMock?: ReturnType) { - const activeFetchMock = fetchMock ?? makeUnexpectedFetchMock(); - global.fetch = withFetchPreconnect(activeFetchMock); - const state = makeState("remote"); - const ctx = createBrowserRouteContext({ getState: () => state }); - return { state, remote: ctx.forProfile("remote"), fetchMock: activeFetchMock }; -} - -function createSequentialPageLister(responses: T[]) { - return vi.fn(async () => { - const next = responses.shift(); - if (!next) { - throw new Error("no more responses"); - } - return next; - }); -} - -type JsonListEntry = { - id: string; - title: string; - url: string; - webSocketDebuggerUrl: string; - type: "page"; -}; - -function createJsonListFetchMock(entries: JsonListEntry[]) { - return vi.fn(async (url: unknown) => { - const u = String(url); - if (!u.includes("/json/list")) { - throw new Error(`unexpected fetch: ${u}`); - } - return { - ok: true, - json: async () => entries, - } as unknown as Response; - }); -} - -function makeManagedTab(id: string, ordinal: number): JsonListEntry { - return { - id, - title: String(ordinal), - url: `http://127.0.0.1:300${ordinal}`, - webSocketDebuggerUrl: `ws://127.0.0.1/devtools/page/${id}`, - type: "page", - }; -} - -function makeManagedTabsWithNew(params?: { newFirst?: boolean }): JsonListEntry[] { - const oldTabs = Array.from({ length: 8 }, (_, index) => - makeManagedTab(`OLD${index + 1}`, index + 1), - ); - const newTab = makeManagedTab("NEW", 9); - return params?.newFirst ? [newTab, ...oldTabs] : [...oldTabs, newTab]; -} - -describe("browser server-context remote profile tab operations", () => { - it("uses profile-level attachOnly when global attachOnly is false", async () => { - const state = makeState("openclaw"); - state.resolved.attachOnly = false; - state.resolved.profiles.openclaw = { - cdpPort: 18800, - attachOnly: true, - color: "#FF4500", - }; - - const reachableMock = vi.mocked(chromeModule.isChromeReachable).mockResolvedValueOnce(false); - const launchMock = vi.mocked(chromeModule.launchOpenClawChrome); - const ctx = createBrowserRouteContext({ getState: () => state }); - - await expect(ctx.forProfile("openclaw").ensureBrowserAvailable()).rejects.toThrow( - /attachOnly is enabled/i, - ); - expect(reachableMock).toHaveBeenCalled(); - expect(launchMock).not.toHaveBeenCalled(); - }); - - it("keeps attachOnly websocket failures off the loopback ownership error path", async () => { - const state = makeState("openclaw"); - state.resolved.attachOnly = false; - state.resolved.profiles.openclaw = { - cdpPort: 18800, - attachOnly: true, - color: "#FF4500", - }; - - const httpReachableMock = vi.mocked(chromeModule.isChromeReachable).mockResolvedValueOnce(true); - const wsReachableMock = vi.mocked(chromeModule.isChromeCdpReady).mockResolvedValueOnce(false); - const launchMock = vi.mocked(chromeModule.launchOpenClawChrome); - const ctx = createBrowserRouteContext({ getState: () => state }); - - await expect(ctx.forProfile("openclaw").ensureBrowserAvailable()).rejects.toThrow( - /attachOnly is enabled and CDP websocket/i, - ); - expect(httpReachableMock).toHaveBeenCalled(); - expect(wsReachableMock).toHaveBeenCalled(); - expect(launchMock).not.toHaveBeenCalled(); - }); - - it("uses Playwright tab operations when available", async () => { - const listPagesViaPlaywright = vi.fn(async () => [ - { targetId: "T1", title: "Tab 1", url: "https://example.com", type: "page" }, - ]); - const createPageViaPlaywright = vi.fn(async () => ({ - targetId: "T2", - title: "Tab 2", - url: "http://127.0.0.1:3000", - type: "page", - })); - const closePageByTargetIdViaPlaywright = vi.fn(async () => {}); - - vi.spyOn(pwAiModule, "getPwAiModule").mockResolvedValue({ - listPagesViaPlaywright, - createPageViaPlaywright, - closePageByTargetIdViaPlaywright, - } as unknown as Awaited>); - - const { state, remote, fetchMock } = createRemoteRouteHarness(); - - const tabs = await remote.listTabs(); - expect(tabs.map((t) => t.targetId)).toEqual(["T1"]); - - const opened = await remote.openTab("http://127.0.0.1:3000"); - expect(opened.targetId).toBe("T2"); - expect(state.profiles.get("remote")?.lastTargetId).toBe("T2"); - expect(createPageViaPlaywright).toHaveBeenCalledWith({ - cdpUrl: "https://browserless.example/chrome?token=abc", - url: "http://127.0.0.1:3000", - ssrfPolicy: { allowPrivateNetwork: true }, - }); - - await remote.closeTab("T1"); - expect(closePageByTargetIdViaPlaywright).toHaveBeenCalledWith({ - cdpUrl: "https://browserless.example/chrome?token=abc", - targetId: "T1", - }); - expect(fetchMock).not.toHaveBeenCalled(); - }); - - it("prefers lastTargetId for remote profiles when targetId is omitted", async () => { - const responses = [ - // ensureTabAvailable() calls listTabs twice - [ - { targetId: "A", title: "A", url: "https://example.com", type: "page" }, - { targetId: "B", title: "B", url: "https://www.example.com", type: "page" }, - ], - [ - { targetId: "A", title: "A", url: "https://example.com", type: "page" }, - { targetId: "B", title: "B", url: "https://www.example.com", type: "page" }, - ], - // second ensureTabAvailable() calls listTabs twice, order flips - [ - { targetId: "B", title: "B", url: "https://www.example.com", type: "page" }, - { targetId: "A", title: "A", url: "https://example.com", type: "page" }, - ], - [ - { targetId: "B", title: "B", url: "https://www.example.com", type: "page" }, - { targetId: "A", title: "A", url: "https://example.com", type: "page" }, - ], - ]; - - const listPagesViaPlaywright = vi.fn(async () => { - const next = responses.shift(); - if (!next) { - throw new Error("no more responses"); - } - return next; - }); - - vi.spyOn(pwAiModule, "getPwAiModule").mockResolvedValue({ - listPagesViaPlaywright, - createPageViaPlaywright: vi.fn(async () => { - throw new Error("unexpected create"); - }), - closePageByTargetIdViaPlaywright: vi.fn(async () => { - throw new Error("unexpected close"); - }), - } as unknown as Awaited>); - - const { remote } = createRemoteRouteHarness(); - - const first = await remote.ensureTabAvailable(); - expect(first.targetId).toBe("A"); - const second = await remote.ensureTabAvailable(); - expect(second.targetId).toBe("A"); - }); - - it("falls back to the only tab for remote profiles when targetId is stale", async () => { - const responses = [ - [{ targetId: "T1", title: "Tab 1", url: "https://example.com", type: "page" }], - [{ targetId: "T1", title: "Tab 1", url: "https://example.com", type: "page" }], - ]; - const listPagesViaPlaywright = createSequentialPageLister(responses); - - vi.spyOn(pwAiModule, "getPwAiModule").mockResolvedValue({ - listPagesViaPlaywright, - } as unknown as Awaited>); - - const { remote } = createRemoteRouteHarness(); - const chosen = await remote.ensureTabAvailable("STALE_TARGET"); - expect(chosen.targetId).toBe("T1"); - }); - - it("keeps rejecting stale targetId for remote profiles when multiple tabs exist", async () => { - const responses = [ - [ - { targetId: "A", title: "A", url: "https://a.example", type: "page" }, - { targetId: "B", title: "B", url: "https://b.example", type: "page" }, - ], - [ - { targetId: "A", title: "A", url: "https://a.example", type: "page" }, - { targetId: "B", title: "B", url: "https://b.example", type: "page" }, - ], - ]; - const listPagesViaPlaywright = createSequentialPageLister(responses); - - vi.spyOn(pwAiModule, "getPwAiModule").mockResolvedValue({ - listPagesViaPlaywright, - } as unknown as Awaited>); - - const { remote } = createRemoteRouteHarness(); - await expect(remote.ensureTabAvailable("STALE_TARGET")).rejects.toThrow(/tab not found/i); - }); - - it("uses Playwright focus for remote profiles when available", async () => { - const listPagesViaPlaywright = vi.fn(async () => [ - { targetId: "T1", title: "Tab 1", url: "https://example.com", type: "page" }, - ]); - const focusPageByTargetIdViaPlaywright = vi.fn(async () => {}); - - vi.spyOn(pwAiModule, "getPwAiModule").mockResolvedValue({ - listPagesViaPlaywright, - focusPageByTargetIdViaPlaywright, - } as unknown as Awaited>); - - const { state, remote, fetchMock } = createRemoteRouteHarness(); - - await remote.focusTab("T1"); - expect(focusPageByTargetIdViaPlaywright).toHaveBeenCalledWith({ - cdpUrl: "https://browserless.example/chrome?token=abc", - targetId: "T1", - }); - expect(fetchMock).not.toHaveBeenCalled(); - expect(state.profiles.get("remote")?.lastTargetId).toBe("T1"); - }); - - it("does not swallow Playwright runtime errors for remote profiles", async () => { - vi.spyOn(pwAiModule, "getPwAiModule").mockResolvedValue({ - listPagesViaPlaywright: vi.fn(async () => { - throw new Error("boom"); - }), - } as unknown as Awaited>); - - const { remote, fetchMock } = createRemoteRouteHarness(); - - await expect(remote.listTabs()).rejects.toThrow(/boom/); - expect(fetchMock).not.toHaveBeenCalled(); - }); - - it("falls back to /json/list when Playwright is not available", async () => { - vi.spyOn(pwAiModule, "getPwAiModule").mockResolvedValue(null); - - const fetchMock = createJsonListFetchMock([ - { - id: "T1", - title: "Tab 1", - url: "https://example.com", - webSocketDebuggerUrl: "wss://browserless.example/devtools/page/T1", - type: "page", - }, - ]); - - const { remote } = createRemoteRouteHarness(fetchMock); - - const tabs = await remote.listTabs(); - expect(tabs.map((t) => t.targetId)).toEqual(["T1"]); - expect(fetchMock).toHaveBeenCalledTimes(1); - }); - - it("does not enforce managed tab cap for remote openclaw profiles", async () => { - const listPagesViaPlaywright = vi - .fn() - .mockResolvedValueOnce([ - { targetId: "T1", title: "1", url: "https://1.example", type: "page" }, - ]) - .mockResolvedValueOnce([ - { targetId: "T1", title: "1", url: "https://1.example", type: "page" }, - { targetId: "T2", title: "2", url: "https://2.example", type: "page" }, - { targetId: "T3", title: "3", url: "https://3.example", type: "page" }, - { targetId: "T4", title: "4", url: "https://4.example", type: "page" }, - { targetId: "T5", title: "5", url: "https://5.example", type: "page" }, - { targetId: "T6", title: "6", url: "https://6.example", type: "page" }, - { targetId: "T7", title: "7", url: "https://7.example", type: "page" }, - { targetId: "T8", title: "8", url: "https://8.example", type: "page" }, - { targetId: "T9", title: "9", url: "https://9.example", type: "page" }, - ]); - - const createPageViaPlaywright = vi.fn(async () => ({ - targetId: "T1", - title: "Tab 1", - url: "https://1.example", - type: "page", - })); - - vi.spyOn(pwAiModule, "getPwAiModule").mockResolvedValue({ - listPagesViaPlaywright, - createPageViaPlaywright, - } as unknown as Awaited>); - - const fetchMock = vi.fn(async (url: unknown) => { - throw new Error(`unexpected fetch: ${String(url)}`); - }); - - const { remote } = createRemoteRouteHarness(fetchMock); - const opened = await remote.openTab("https://1.example"); - expect(opened.targetId).toBe("T1"); - expect(fetchMock).not.toHaveBeenCalled(); - }); -}); - -describe("browser server-context tab selection state", () => { - it("updates lastTargetId when openTab is created via CDP", async () => { - const createTargetViaCdp = vi - .spyOn(cdpModule, "createTargetViaCdp") - .mockResolvedValue({ targetId: "CREATED" }); - - const fetchMock = createJsonListFetchMock([ - { - id: "CREATED", - title: "New Tab", - url: "http://127.0.0.1:8080", - webSocketDebuggerUrl: "ws://127.0.0.1/devtools/page/CREATED", - type: "page", - }, - ]); - - global.fetch = withFetchPreconnect(fetchMock); - - const state = makeState("openclaw"); - const ctx = createBrowserRouteContext({ getState: () => state }); - const openclaw = ctx.forProfile("openclaw"); - - const opened = await openclaw.openTab("http://127.0.0.1:8080"); - expect(opened.targetId).toBe("CREATED"); - expect(state.profiles.get("openclaw")?.lastTargetId).toBe("CREATED"); - expect(createTargetViaCdp).toHaveBeenCalledWith({ - cdpUrl: "http://127.0.0.1:18800", - url: "http://127.0.0.1:8080", - ssrfPolicy: { allowPrivateNetwork: true }, - }); - }); - - it("closes excess managed tabs after opening a new tab", async () => { - vi.spyOn(cdpModule, "createTargetViaCdp").mockResolvedValue({ targetId: "NEW" }); - - const existingTabs = makeManagedTabsWithNew(); - - const fetchMock = vi.fn(async (url: unknown) => { - const value = String(url); - if (value.includes("/json/list")) { - return { ok: true, json: async () => existingTabs } as unknown as Response; - } - if (value.includes("/json/close/OLD1")) { - return { ok: true, json: async () => ({}) } as unknown as Response; - } - throw new Error(`unexpected fetch: ${value}`); - }); - - global.fetch = withFetchPreconnect(fetchMock); - const state = makeState("openclaw"); - (state.profiles as Map).set("openclaw", { - profile: { name: "openclaw" }, - running: { pid: 1234, proc: { on: vi.fn() } }, - lastTargetId: null, - }); - const ctx = createBrowserRouteContext({ getState: () => state }); - const openclaw = ctx.forProfile("openclaw"); - - const opened = await openclaw.openTab("http://127.0.0.1:3009"); - expect(opened.targetId).toBe("NEW"); - await vi.waitFor(() => { - expect(fetchMock).toHaveBeenCalledWith( - expect.stringContaining("/json/close/OLD1"), - expect.any(Object), - ); - }); - }); - - it("never closes the just-opened managed tab during cap cleanup", async () => { - vi.spyOn(cdpModule, "createTargetViaCdp").mockResolvedValue({ targetId: "NEW" }); - - const existingTabs = makeManagedTabsWithNew({ newFirst: true }); - - const fetchMock = vi.fn(async (url: unknown) => { - const value = String(url); - if (value.includes("/json/list")) { - return { ok: true, json: async () => existingTabs } as unknown as Response; - } - if (value.includes("/json/close/OLD1")) { - return { ok: true, json: async () => ({}) } as unknown as Response; - } - if (value.includes("/json/close/NEW")) { - throw new Error("cleanup must not close NEW"); - } - throw new Error(`unexpected fetch: ${value}`); - }); - - global.fetch = withFetchPreconnect(fetchMock); - const state = makeState("openclaw"); - (state.profiles as Map).set("openclaw", { - profile: { name: "openclaw" }, - running: { pid: 1234, proc: { on: vi.fn() } }, - lastTargetId: null, - }); - const ctx = createBrowserRouteContext({ getState: () => state }); - const openclaw = ctx.forProfile("openclaw"); - - const opened = await openclaw.openTab("http://127.0.0.1:3009"); - expect(opened.targetId).toBe("NEW"); - await vi.waitFor(() => { - expect(fetchMock).toHaveBeenCalledWith( - expect.stringContaining("/json/close/OLD1"), - expect.any(Object), - ); - }); - expect(fetchMock).not.toHaveBeenCalledWith( - expect.stringContaining("/json/close/NEW"), - expect.anything(), - ); - }); - - it("does not fail tab open when managed-tab cleanup list fails", async () => { - vi.spyOn(cdpModule, "createTargetViaCdp").mockResolvedValue({ targetId: "NEW" }); - - let listCount = 0; - const fetchMock = vi.fn(async (url: unknown) => { - const value = String(url); - if (value.includes("/json/list")) { - listCount += 1; - if (listCount === 1) { - return { - ok: true, - json: async () => [ - { - id: "NEW", - title: "New Tab", - url: "http://127.0.0.1:3009", - webSocketDebuggerUrl: "ws://127.0.0.1/devtools/page/NEW", - type: "page", - }, - ], - } as unknown as Response; - } - throw new Error("/json/list timeout"); - } - throw new Error(`unexpected fetch: ${value}`); - }); - - global.fetch = withFetchPreconnect(fetchMock); - const state = makeState("openclaw"); - (state.profiles as Map).set("openclaw", { - profile: { name: "openclaw" }, - running: { pid: 1234, proc: { on: vi.fn() } }, - lastTargetId: null, - }); - const ctx = createBrowserRouteContext({ getState: () => state }); - const openclaw = ctx.forProfile("openclaw"); - - const opened = await openclaw.openTab("http://127.0.0.1:3009"); - expect(opened.targetId).toBe("NEW"); - }); - - it("does not run managed tab cleanup in attachOnly mode", async () => { - vi.spyOn(cdpModule, "createTargetViaCdp").mockResolvedValue({ targetId: "NEW" }); - - const existingTabs = makeManagedTabsWithNew(); - - const fetchMock = vi.fn(async (url: unknown) => { - const value = String(url); - if (value.includes("/json/list")) { - return { ok: true, json: async () => existingTabs } as unknown as Response; - } - if (value.includes("/json/close/")) { - throw new Error("should not close tabs in attachOnly mode"); - } - throw new Error(`unexpected fetch: ${value}`); - }); - - global.fetch = withFetchPreconnect(fetchMock); - const state = makeState("openclaw"); - state.resolved.attachOnly = true; - const ctx = createBrowserRouteContext({ getState: () => state }); - const openclaw = ctx.forProfile("openclaw"); - - const opened = await openclaw.openTab("http://127.0.0.1:3009"); - expect(opened.targetId).toBe("NEW"); - expect(fetchMock).not.toHaveBeenCalledWith( - expect.stringContaining("/json/close/"), - expect.anything(), - ); - }); - - it("does not block openTab on slow best-effort cleanup closes", async () => { - vi.spyOn(cdpModule, "createTargetViaCdp").mockResolvedValue({ targetId: "NEW" }); - - const existingTabs = makeManagedTabsWithNew(); - - const fetchMock = vi.fn(async (url: unknown) => { - const value = String(url); - if (value.includes("/json/list")) { - return { ok: true, json: async () => existingTabs } as unknown as Response; - } - if (value.includes("/json/close/OLD1")) { - return new Promise(() => {}); - } - throw new Error(`unexpected fetch: ${value}`); - }); - - global.fetch = withFetchPreconnect(fetchMock); - const state = makeState("openclaw"); - (state.profiles as Map).set("openclaw", { - profile: { name: "openclaw" }, - running: { pid: 1234, proc: { on: vi.fn() } }, - lastTargetId: null, - }); - const ctx = createBrowserRouteContext({ getState: () => state }); - const openclaw = ctx.forProfile("openclaw"); - - const opened = await Promise.race([ - openclaw.openTab("http://127.0.0.1:3009"), - new Promise((_, reject) => - setTimeout(() => reject(new Error("openTab timed out waiting for cleanup")), 300), - ), - ]); - - expect(opened.targetId).toBe("NEW"); - }); - - it("blocks unsupported non-network URLs before any HTTP tab-open fallback", async () => { - const fetchMock = vi.fn(async () => { - throw new Error("unexpected fetch"); - }); - - global.fetch = withFetchPreconnect(fetchMock); - const state = makeState("openclaw"); - const ctx = createBrowserRouteContext({ getState: () => state }); - const openclaw = ctx.forProfile("openclaw"); - - await expect(openclaw.openTab("file:///etc/passwd")).rejects.toBeInstanceOf( - InvalidBrowserNavigationUrlError, - ); - expect(fetchMock).not.toHaveBeenCalled(); - }); -}); diff --git a/src/browser/server-context.reset.test.ts b/src/browser/server-context.reset.test.ts new file mode 100644 index 00000000000..1796fa3f68b --- /dev/null +++ b/src/browser/server-context.reset.test.ts @@ -0,0 +1,145 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { createProfileResetOps } from "./server-context.reset.js"; + +const relayMocks = vi.hoisted(() => ({ + stopChromeExtensionRelayServer: vi.fn(async () => true), +})); + +const trashMocks = vi.hoisted(() => ({ + movePathToTrash: vi.fn(async (from: string) => `${from}.trashed`), +})); + +const pwAiMocks = vi.hoisted(() => ({ + closePlaywrightBrowserConnection: vi.fn(async () => {}), +})); + +vi.mock("./extension-relay.js", () => relayMocks); +vi.mock("./trash.js", () => trashMocks); +vi.mock("./pw-ai.js", () => pwAiMocks); + +afterEach(() => { + vi.clearAllMocks(); +}); + +describe("createProfileResetOps", () => { + it("stops extension relay for extension profiles", async () => { + const ops = createProfileResetOps({ + profile: { + name: "chrome", + cdpUrl: "http://127.0.0.1:18800", + cdpHost: "127.0.0.1", + cdpIsLoopback: true, + cdpPort: 18800, + color: "#f60", + driver: "extension", + attachOnly: false, + }, + getProfileState: () => ({ profile: {} as never, running: null }), + stopRunningBrowser: vi.fn(async () => ({ stopped: false })), + isHttpReachable: vi.fn(async () => false), + resolveOpenClawUserDataDir: (name: string) => `/tmp/${name}`, + }); + + await expect(ops.resetProfile()).resolves.toEqual({ + moved: false, + from: "http://127.0.0.1:18800", + }); + expect(relayMocks.stopChromeExtensionRelayServer).toHaveBeenCalledWith({ + cdpUrl: "http://127.0.0.1:18800", + }); + expect(trashMocks.movePathToTrash).not.toHaveBeenCalled(); + }); + + it("rejects remote non-extension profiles", async () => { + const ops = createProfileResetOps({ + profile: { + name: "remote", + cdpUrl: "https://browserless.example/chrome", + cdpHost: "browserless.example", + cdpIsLoopback: false, + cdpPort: 443, + color: "#0f0", + driver: "openclaw", + attachOnly: false, + }, + getProfileState: () => ({ profile: {} as never, running: null }), + stopRunningBrowser: vi.fn(async () => ({ stopped: false })), + isHttpReachable: vi.fn(async () => false), + resolveOpenClawUserDataDir: (name: string) => `/tmp/${name}`, + }); + + await expect(ops.resetProfile()).rejects.toThrow(/only supported for local profiles/i); + }); + + it("stops local browser, closes playwright connection, and trashes profile dir", async () => { + const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-reset-")); + const profileDir = path.join(tempRoot, "openclaw"); + fs.mkdirSync(profileDir, { recursive: true }); + + const stopRunningBrowser = vi.fn(async () => ({ stopped: true })); + const isHttpReachable = vi.fn(async () => true); + const getProfileState = vi.fn(() => ({ + profile: {} as never, + running: { pid: 1 } as never, + })); + + const ops = createProfileResetOps({ + profile: { + name: "openclaw", + cdpUrl: "http://127.0.0.1:18800", + cdpHost: "127.0.0.1", + cdpIsLoopback: true, + cdpPort: 18800, + color: "#f60", + driver: "openclaw", + attachOnly: false, + }, + getProfileState, + stopRunningBrowser, + isHttpReachable, + resolveOpenClawUserDataDir: () => profileDir, + }); + + const result = await ops.resetProfile(); + expect(result).toEqual({ + moved: true, + from: profileDir, + to: `${profileDir}.trashed`, + }); + expect(isHttpReachable).toHaveBeenCalledWith(300); + expect(stopRunningBrowser).toHaveBeenCalledTimes(1); + expect(pwAiMocks.closePlaywrightBrowserConnection).toHaveBeenCalledTimes(1); + expect(trashMocks.movePathToTrash).toHaveBeenCalledWith(profileDir); + }); + + it("forces playwright disconnect when loopback cdp is occupied by non-owned process", async () => { + const tempRoot = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-reset-no-own-")); + const profileDir = path.join(tempRoot, "openclaw"); + fs.mkdirSync(profileDir, { recursive: true }); + + const stopRunningBrowser = vi.fn(async () => ({ stopped: false })); + const ops = createProfileResetOps({ + profile: { + name: "openclaw", + cdpUrl: "http://127.0.0.1:18800", + cdpHost: "127.0.0.1", + cdpIsLoopback: true, + cdpPort: 18800, + color: "#f60", + driver: "openclaw", + attachOnly: false, + }, + getProfileState: () => ({ profile: {} as never, running: null }), + stopRunningBrowser, + isHttpReachable: vi.fn(async () => true), + resolveOpenClawUserDataDir: () => profileDir, + }); + + await ops.resetProfile(); + expect(stopRunningBrowser).not.toHaveBeenCalled(); + expect(pwAiMocks.closePlaywrightBrowserConnection).toHaveBeenCalledTimes(2); + }); +}); diff --git a/src/browser/server-context.reset.ts b/src/browser/server-context.reset.ts new file mode 100644 index 00000000000..134db475f61 --- /dev/null +++ b/src/browser/server-context.reset.ts @@ -0,0 +1,69 @@ +import fs from "node:fs"; +import type { ResolvedBrowserProfile } from "./config.js"; +import { stopChromeExtensionRelayServer } from "./extension-relay.js"; +import type { ProfileRuntimeState } from "./server-context.types.js"; +import { movePathToTrash } from "./trash.js"; + +type ResetDeps = { + profile: ResolvedBrowserProfile; + getProfileState: () => ProfileRuntimeState; + stopRunningBrowser: () => Promise<{ stopped: boolean }>; + isHttpReachable: (timeoutMs?: number) => Promise; + resolveOpenClawUserDataDir: (profileName: string) => string; +}; + +type ResetOps = { + resetProfile: () => Promise<{ moved: boolean; from: string; to?: string }>; +}; + +async function closePlaywrightBrowserConnection(): Promise { + try { + const mod = await import("./pw-ai.js"); + await mod.closePlaywrightBrowserConnection(); + } catch { + // ignore + } +} + +export function createProfileResetOps({ + profile, + getProfileState, + stopRunningBrowser, + isHttpReachable, + resolveOpenClawUserDataDir, +}: ResetDeps): ResetOps { + const resetProfile = async () => { + if (profile.driver === "extension") { + await stopChromeExtensionRelayServer({ cdpUrl: profile.cdpUrl }).catch(() => {}); + return { moved: false, from: profile.cdpUrl }; + } + if (!profile.cdpIsLoopback) { + throw new Error( + `reset-profile is only supported for local profiles (profile "${profile.name}" is remote).`, + ); + } + + const userDataDir = resolveOpenClawUserDataDir(profile.name); + const profileState = getProfileState(); + const httpReachable = await isHttpReachable(300); + if (httpReachable && !profileState.running) { + // Port in use but not by us - kill it. + await closePlaywrightBrowserConnection(); + } + + if (profileState.running) { + await stopRunningBrowser(); + } + + await closePlaywrightBrowserConnection(); + + if (!fs.existsSync(userDataDir)) { + return { moved: false, from: userDataDir }; + } + + const moved = await movePathToTrash(userDataDir); + return { moved: true, from: userDataDir, to: moved }; + }; + + return { resetProfile }; +} diff --git a/src/browser/server-context.selection.ts b/src/browser/server-context.selection.ts new file mode 100644 index 00000000000..e1c78426eab --- /dev/null +++ b/src/browser/server-context.selection.ts @@ -0,0 +1,155 @@ +import { fetchOk } from "./cdp.helpers.js"; +import { appendCdpPath } from "./cdp.js"; +import type { ResolvedBrowserProfile } from "./config.js"; +import type { PwAiModule } from "./pw-ai-module.js"; +import { getPwAiModule } from "./pw-ai-module.js"; +import type { BrowserTab, ProfileRuntimeState } from "./server-context.types.js"; +import { resolveTargetIdFromTabs } from "./target-id.js"; + +type SelectionDeps = { + profile: ResolvedBrowserProfile; + getProfileState: () => ProfileRuntimeState; + ensureBrowserAvailable: () => Promise; + listTabs: () => Promise; + openTab: (url: string) => Promise; +}; + +type SelectionOps = { + ensureTabAvailable: (targetId?: string) => Promise; + focusTab: (targetId: string) => Promise; + closeTab: (targetId: string) => Promise; +}; + +export function createProfileSelectionOps({ + profile, + getProfileState, + ensureBrowserAvailable, + listTabs, + openTab, +}: SelectionDeps): SelectionOps { + const ensureTabAvailable = async (targetId?: string): Promise => { + await ensureBrowserAvailable(); + const profileState = getProfileState(); + const tabs1 = await listTabs(); + if (tabs1.length === 0) { + if (profile.driver === "extension") { + throw new Error( + `tab not found (no attached Chrome tabs for profile "${profile.name}"). ` + + "Click the OpenClaw Browser Relay toolbar icon on the tab you want to control (badge ON).", + ); + } + await openTab("about:blank"); + } + + const tabs = await listTabs(); + // For remote profiles using Playwright's persistent connection, we don't need wsUrl + // because we access pages directly through Playwright, not via individual WebSocket URLs. + const candidates = + profile.driver === "extension" || !profile.cdpIsLoopback + ? tabs + : tabs.filter((t) => Boolean(t.wsUrl)); + + const resolveById = (raw: string) => { + const resolved = resolveTargetIdFromTabs(raw, candidates); + if (!resolved.ok) { + if (resolved.reason === "ambiguous") { + return "AMBIGUOUS" as const; + } + return null; + } + return candidates.find((t) => t.targetId === resolved.targetId) ?? null; + }; + + const pickDefault = () => { + const last = profileState.lastTargetId?.trim() || ""; + const lastResolved = last ? resolveById(last) : null; + if (lastResolved && lastResolved !== "AMBIGUOUS") { + return lastResolved; + } + // Prefer a real page tab first (avoid service workers/background targets). + const page = candidates.find((t) => (t.type ?? "page") === "page"); + return page ?? candidates.at(0) ?? null; + }; + + let chosen = targetId ? resolveById(targetId) : pickDefault(); + if ( + !chosen && + (profile.driver === "extension" || !profile.cdpIsLoopback) && + candidates.length === 1 + ) { + // If an agent passes a stale/foreign targetId but only one candidate remains, + // recover by using that tab instead of failing hard. + chosen = candidates[0] ?? null; + } + + if (chosen === "AMBIGUOUS") { + throw new Error("ambiguous target id prefix"); + } + if (!chosen) { + throw new Error("tab not found"); + } + profileState.lastTargetId = chosen.targetId; + return chosen; + }; + + const resolveTargetIdOrThrow = async (targetId: string): Promise => { + const tabs = await listTabs(); + const resolved = resolveTargetIdFromTabs(targetId, tabs); + if (!resolved.ok) { + if (resolved.reason === "ambiguous") { + throw new Error("ambiguous target id prefix"); + } + throw new Error("tab not found"); + } + return resolved.targetId; + }; + + const focusTab = async (targetId: string): Promise => { + const resolvedTargetId = await resolveTargetIdOrThrow(targetId); + + if (!profile.cdpIsLoopback) { + const mod = await getPwAiModule({ mode: "strict" }); + const focusPageByTargetIdViaPlaywright = (mod as Partial | null) + ?.focusPageByTargetIdViaPlaywright; + if (typeof focusPageByTargetIdViaPlaywright === "function") { + await focusPageByTargetIdViaPlaywright({ + cdpUrl: profile.cdpUrl, + targetId: resolvedTargetId, + }); + const profileState = getProfileState(); + profileState.lastTargetId = resolvedTargetId; + return; + } + } + + await fetchOk(appendCdpPath(profile.cdpUrl, `/json/activate/${resolvedTargetId}`)); + const profileState = getProfileState(); + profileState.lastTargetId = resolvedTargetId; + }; + + const closeTab = async (targetId: string): Promise => { + const resolvedTargetId = await resolveTargetIdOrThrow(targetId); + + // For remote profiles, use Playwright's persistent connection to close tabs + if (!profile.cdpIsLoopback) { + const mod = await getPwAiModule({ mode: "strict" }); + const closePageByTargetIdViaPlaywright = (mod as Partial | null) + ?.closePageByTargetIdViaPlaywright; + if (typeof closePageByTargetIdViaPlaywright === "function") { + await closePageByTargetIdViaPlaywright({ + cdpUrl: profile.cdpUrl, + targetId: resolvedTargetId, + }); + return; + } + } + + await fetchOk(appendCdpPath(profile.cdpUrl, `/json/close/${resolvedTargetId}`)); + }; + + return { + ensureTabAvailable, + focusTab, + closeTab, + }; +} diff --git a/src/browser/server-context.tab-ops.ts b/src/browser/server-context.tab-ops.ts index b1c307369d6..cf026d658a7 100644 --- a/src/browser/server-context.tab-ops.ts +++ b/src/browser/server-context.tab-ops.ts @@ -1,3 +1,4 @@ +import { CDP_JSON_NEW_TIMEOUT_MS } from "./cdp-timeouts.js"; import { fetchJson, fetchOk } from "./cdp.helpers.js"; import { appendCdpPath, createTargetViaCdp, normalizeCdpWsUrl } from "./cdp.js"; import type { ResolvedBrowserProfile } from "./config.js"; @@ -187,11 +188,11 @@ export function createProfileTabOps({ return endpointUrl.toString(); })() : `${endpointUrl.toString()}?${encoded}`; - const created = await fetchJson(endpoint, 1500, { + const created = await fetchJson(endpoint, CDP_JSON_NEW_TIMEOUT_MS, { method: "PUT", }).catch(async (err) => { if (String(err).includes("HTTP 405")) { - return await fetchJson(endpoint, 1500); + return await fetchJson(endpoint, CDP_JSON_NEW_TIMEOUT_MS); } throw err; }); diff --git a/src/browser/server-context.tab-selection-state.test.ts b/src/browser/server-context.tab-selection-state.test.ts new file mode 100644 index 00000000000..a0d602074f4 --- /dev/null +++ b/src/browser/server-context.tab-selection-state.test.ts @@ -0,0 +1,255 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { withFetchPreconnect } from "../test-utils/fetch-mock.js"; +import "./server-context.chrome-test-harness.js"; +import * as cdpModule from "./cdp.js"; +import { InvalidBrowserNavigationUrlError } from "./navigation-guard.js"; +import { createBrowserRouteContext } from "./server-context.js"; +import { + makeManagedTabsWithNew, + makeState, + originalFetch, +} from "./server-context.remote-tab-ops.harness.js"; + +afterEach(() => { + globalThis.fetch = originalFetch; + vi.restoreAllMocks(); +}); + +describe("browser server-context tab selection state", () => { + it("updates lastTargetId when openTab is created via CDP", async () => { + const createTargetViaCdp = vi + .spyOn(cdpModule, "createTargetViaCdp") + .mockResolvedValue({ targetId: "CREATED" }); + + const fetchMock = vi.fn(async (url: unknown) => { + const u = String(url); + if (!u.includes("/json/list")) { + throw new Error(`unexpected fetch: ${u}`); + } + return { + ok: true, + json: async () => [ + { + id: "CREATED", + title: "New Tab", + url: "http://127.0.0.1:8080", + webSocketDebuggerUrl: "ws://127.0.0.1/devtools/page/CREATED", + type: "page", + }, + ], + } as unknown as Response; + }); + + global.fetch = withFetchPreconnect(fetchMock); + const state = makeState("openclaw"); + const ctx = createBrowserRouteContext({ getState: () => state }); + const openclaw = ctx.forProfile("openclaw"); + + const opened = await openclaw.openTab("http://127.0.0.1:8080"); + expect(opened.targetId).toBe("CREATED"); + expect(state.profiles.get("openclaw")?.lastTargetId).toBe("CREATED"); + expect(createTargetViaCdp).toHaveBeenCalledWith({ + cdpUrl: "http://127.0.0.1:18800", + url: "http://127.0.0.1:8080", + ssrfPolicy: { allowPrivateNetwork: true }, + }); + }); + + it("closes excess managed tabs after opening a new tab", async () => { + vi.spyOn(cdpModule, "createTargetViaCdp").mockResolvedValue({ targetId: "NEW" }); + const existingTabs = makeManagedTabsWithNew(); + + const fetchMock = vi.fn(async (url: unknown) => { + const value = String(url); + if (value.includes("/json/list")) { + return { ok: true, json: async () => existingTabs } as unknown as Response; + } + if (value.includes("/json/close/OLD1")) { + return { ok: true, json: async () => ({}) } as unknown as Response; + } + throw new Error(`unexpected fetch: ${value}`); + }); + + global.fetch = withFetchPreconnect(fetchMock); + const state = makeState("openclaw"); + (state.profiles as Map).set("openclaw", { + profile: { name: "openclaw" }, + running: { pid: 1234, proc: { on: vi.fn() } }, + lastTargetId: null, + }); + const ctx = createBrowserRouteContext({ getState: () => state }); + const openclaw = ctx.forProfile("openclaw"); + + const opened = await openclaw.openTab("http://127.0.0.1:3009"); + expect(opened.targetId).toBe("NEW"); + await vi.waitFor(() => { + expect(fetchMock).toHaveBeenCalledWith( + expect.stringContaining("/json/close/OLD1"), + expect.any(Object), + ); + }); + }); + + it("never closes the just-opened managed tab during cap cleanup", async () => { + vi.spyOn(cdpModule, "createTargetViaCdp").mockResolvedValue({ targetId: "NEW" }); + const existingTabs = makeManagedTabsWithNew({ newFirst: true }); + + const fetchMock = vi.fn(async (url: unknown) => { + const value = String(url); + if (value.includes("/json/list")) { + return { ok: true, json: async () => existingTabs } as unknown as Response; + } + if (value.includes("/json/close/OLD1")) { + return { ok: true, json: async () => ({}) } as unknown as Response; + } + if (value.includes("/json/close/NEW")) { + throw new Error("cleanup must not close NEW"); + } + throw new Error(`unexpected fetch: ${value}`); + }); + + global.fetch = withFetchPreconnect(fetchMock); + const state = makeState("openclaw"); + (state.profiles as Map).set("openclaw", { + profile: { name: "openclaw" }, + running: { pid: 1234, proc: { on: vi.fn() } }, + lastTargetId: null, + }); + const ctx = createBrowserRouteContext({ getState: () => state }); + const openclaw = ctx.forProfile("openclaw"); + + const opened = await openclaw.openTab("http://127.0.0.1:3009"); + expect(opened.targetId).toBe("NEW"); + await vi.waitFor(() => { + expect(fetchMock).toHaveBeenCalledWith( + expect.stringContaining("/json/close/OLD1"), + expect.any(Object), + ); + }); + expect(fetchMock).not.toHaveBeenCalledWith( + expect.stringContaining("/json/close/NEW"), + expect.anything(), + ); + }); + + it("does not fail tab open when managed-tab cleanup list fails", async () => { + vi.spyOn(cdpModule, "createTargetViaCdp").mockResolvedValue({ targetId: "NEW" }); + + let listCount = 0; + const fetchMock = vi.fn(async (url: unknown) => { + const value = String(url); + if (value.includes("/json/list")) { + listCount += 1; + if (listCount === 1) { + return { + ok: true, + json: async () => [ + { + id: "NEW", + title: "New Tab", + url: "http://127.0.0.1:3009", + webSocketDebuggerUrl: "ws://127.0.0.1/devtools/page/NEW", + type: "page", + }, + ], + } as unknown as Response; + } + throw new Error("/json/list timeout"); + } + throw new Error(`unexpected fetch: ${value}`); + }); + + global.fetch = withFetchPreconnect(fetchMock); + const state = makeState("openclaw"); + (state.profiles as Map).set("openclaw", { + profile: { name: "openclaw" }, + running: { pid: 1234, proc: { on: vi.fn() } }, + lastTargetId: null, + }); + const ctx = createBrowserRouteContext({ getState: () => state }); + const openclaw = ctx.forProfile("openclaw"); + + const opened = await openclaw.openTab("http://127.0.0.1:3009"); + expect(opened.targetId).toBe("NEW"); + }); + + it("does not run managed tab cleanup in attachOnly mode", async () => { + vi.spyOn(cdpModule, "createTargetViaCdp").mockResolvedValue({ targetId: "NEW" }); + const existingTabs = makeManagedTabsWithNew(); + + const fetchMock = vi.fn(async (url: unknown) => { + const value = String(url); + if (value.includes("/json/list")) { + return { ok: true, json: async () => existingTabs } as unknown as Response; + } + if (value.includes("/json/close/")) { + throw new Error("should not close tabs in attachOnly mode"); + } + throw new Error(`unexpected fetch: ${value}`); + }); + + global.fetch = withFetchPreconnect(fetchMock); + const state = makeState("openclaw"); + state.resolved.attachOnly = true; + const ctx = createBrowserRouteContext({ getState: () => state }); + const openclaw = ctx.forProfile("openclaw"); + + const opened = await openclaw.openTab("http://127.0.0.1:3009"); + expect(opened.targetId).toBe("NEW"); + expect(fetchMock).not.toHaveBeenCalledWith( + expect.stringContaining("/json/close/"), + expect.anything(), + ); + }); + + it("does not block openTab on slow best-effort cleanup closes", async () => { + vi.spyOn(cdpModule, "createTargetViaCdp").mockResolvedValue({ targetId: "NEW" }); + const existingTabs = makeManagedTabsWithNew(); + + const fetchMock = vi.fn(async (url: unknown) => { + const value = String(url); + if (value.includes("/json/list")) { + return { ok: true, json: async () => existingTabs } as unknown as Response; + } + if (value.includes("/json/close/OLD1")) { + return new Promise(() => {}); + } + throw new Error(`unexpected fetch: ${value}`); + }); + + global.fetch = withFetchPreconnect(fetchMock); + const state = makeState("openclaw"); + (state.profiles as Map).set("openclaw", { + profile: { name: "openclaw" }, + running: { pid: 1234, proc: { on: vi.fn() } }, + lastTargetId: null, + }); + const ctx = createBrowserRouteContext({ getState: () => state }); + const openclaw = ctx.forProfile("openclaw"); + + const opened = await Promise.race([ + openclaw.openTab("http://127.0.0.1:3009"), + new Promise((_, reject) => + setTimeout(() => reject(new Error("openTab timed out waiting for cleanup")), 300), + ), + ]); + + expect(opened.targetId).toBe("NEW"); + }); + + it("blocks unsupported non-network URLs before any HTTP tab-open fallback", async () => { + const fetchMock = vi.fn(async () => { + throw new Error("unexpected fetch"); + }); + + global.fetch = withFetchPreconnect(fetchMock); + const state = makeState("openclaw"); + const ctx = createBrowserRouteContext({ getState: () => state }); + const openclaw = ctx.forProfile("openclaw"); + + await expect(openclaw.openTab("file:///etc/passwd")).rejects.toBeInstanceOf( + InvalidBrowserNavigationUrlError, + ); + expect(fetchMock).not.toHaveBeenCalled(); + }); +}); diff --git a/src/browser/server-context.ts b/src/browser/server-context.ts index 9bf74f45697..29632c7b8a4 100644 --- a/src/browser/server-context.ts +++ b/src/browser/server-context.ts @@ -1,19 +1,15 @@ -import fs from "node:fs"; import { SsrFBlockedError } from "../infra/net/ssrf.js"; -import { fetchOk } from "./cdp.helpers.js"; -import { appendCdpPath } from "./cdp.js"; import { isChromeReachable, resolveOpenClawUserDataDir } from "./chrome.js"; import type { ResolvedBrowserProfile } from "./config.js"; import { resolveProfile } from "./config.js"; -import { stopChromeExtensionRelayServer } from "./extension-relay.js"; import { InvalidBrowserNavigationUrlError } from "./navigation-guard.js"; -import type { PwAiModule } from "./pw-ai-module.js"; -import { getPwAiModule } from "./pw-ai-module.js"; import { refreshResolvedBrowserConfigFromDisk, resolveBrowserProfileWithHotReload, } from "./resolved-config-refresh.js"; import { createProfileAvailability } from "./server-context.availability.js"; +import { createProfileResetOps } from "./server-context.reset.js"; +import { createProfileSelectionOps } from "./server-context.selection.js"; import { createProfileTabOps } from "./server-context.tab-ops.js"; import type { BrowserServerState, @@ -24,8 +20,6 @@ import type { ProfileRuntimeState, ProfileStatus, } from "./server-context.types.js"; -import { resolveTargetIdFromTabs } from "./target-id.js"; -import { movePathToTrash } from "./trash.js"; export type { BrowserRouteContext, @@ -89,168 +83,21 @@ function createProfileContext( setProfileRunning, }); - const ensureTabAvailable = async (targetId?: string): Promise => { - await ensureBrowserAvailable(); - const profileState = getProfileState(); - const tabs1 = await listTabs(); - if (tabs1.length === 0) { - if (profile.driver === "extension") { - throw new Error( - `tab not found (no attached Chrome tabs for profile "${profile.name}"). ` + - "Click the OpenClaw Browser Relay toolbar icon on the tab you want to control (badge ON).", - ); - } - await openTab("about:blank"); - } + const { ensureTabAvailable, focusTab, closeTab } = createProfileSelectionOps({ + profile, + getProfileState, + ensureBrowserAvailable, + listTabs, + openTab, + }); - const tabs = await listTabs(); - // For remote profiles using Playwright's persistent connection, we don't need wsUrl - // because we access pages directly through Playwright, not via individual WebSocket URLs. - const candidates = - profile.driver === "extension" || !profile.cdpIsLoopback - ? tabs - : tabs.filter((t) => Boolean(t.wsUrl)); - - const resolveById = (raw: string) => { - const resolved = resolveTargetIdFromTabs(raw, candidates); - if (!resolved.ok) { - if (resolved.reason === "ambiguous") { - return "AMBIGUOUS" as const; - } - return null; - } - return candidates.find((t) => t.targetId === resolved.targetId) ?? null; - }; - - const pickDefault = () => { - const last = profileState.lastTargetId?.trim() || ""; - const lastResolved = last ? resolveById(last) : null; - if (lastResolved && lastResolved !== "AMBIGUOUS") { - return lastResolved; - } - // Prefer a real page tab first (avoid service workers/background targets). - const page = candidates.find((t) => (t.type ?? "page") === "page"); - return page ?? candidates.at(0) ?? null; - }; - - let chosen = targetId ? resolveById(targetId) : pickDefault(); - if ( - !chosen && - (profile.driver === "extension" || !profile.cdpIsLoopback) && - candidates.length === 1 - ) { - // If an agent passes a stale/foreign targetId but only one candidate remains, - // recover by using that tab instead of failing hard. - chosen = candidates[0] ?? null; - } - - if (chosen === "AMBIGUOUS") { - throw new Error("ambiguous target id prefix"); - } - if (!chosen) { - throw new Error("tab not found"); - } - profileState.lastTargetId = chosen.targetId; - return chosen; - }; - - const resolveTargetIdOrThrow = async (targetId: string): Promise => { - const tabs = await listTabs(); - const resolved = resolveTargetIdFromTabs(targetId, tabs); - if (!resolved.ok) { - if (resolved.reason === "ambiguous") { - throw new Error("ambiguous target id prefix"); - } - throw new Error("tab not found"); - } - return resolved.targetId; - }; - - const focusTab = async (targetId: string): Promise => { - const resolvedTargetId = await resolveTargetIdOrThrow(targetId); - - if (!profile.cdpIsLoopback) { - const mod = await getPwAiModule({ mode: "strict" }); - const focusPageByTargetIdViaPlaywright = (mod as Partial | null) - ?.focusPageByTargetIdViaPlaywright; - if (typeof focusPageByTargetIdViaPlaywright === "function") { - await focusPageByTargetIdViaPlaywright({ - cdpUrl: profile.cdpUrl, - targetId: resolvedTargetId, - }); - const profileState = getProfileState(); - profileState.lastTargetId = resolvedTargetId; - return; - } - } - - await fetchOk(appendCdpPath(profile.cdpUrl, `/json/activate/${resolvedTargetId}`)); - const profileState = getProfileState(); - profileState.lastTargetId = resolvedTargetId; - }; - - const closeTab = async (targetId: string): Promise => { - const resolvedTargetId = await resolveTargetIdOrThrow(targetId); - - // For remote profiles, use Playwright's persistent connection to close tabs - if (!profile.cdpIsLoopback) { - const mod = await getPwAiModule({ mode: "strict" }); - const closePageByTargetIdViaPlaywright = (mod as Partial | null) - ?.closePageByTargetIdViaPlaywright; - if (typeof closePageByTargetIdViaPlaywright === "function") { - await closePageByTargetIdViaPlaywright({ - cdpUrl: profile.cdpUrl, - targetId: resolvedTargetId, - }); - return; - } - } - - await fetchOk(appendCdpPath(profile.cdpUrl, `/json/close/${resolvedTargetId}`)); - }; - - const resetProfile = async () => { - if (profile.driver === "extension") { - await stopChromeExtensionRelayServer({ cdpUrl: profile.cdpUrl }).catch(() => {}); - return { moved: false, from: profile.cdpUrl }; - } - if (!profile.cdpIsLoopback) { - throw new Error( - `reset-profile is only supported for local profiles (profile "${profile.name}" is remote).`, - ); - } - const userDataDir = resolveOpenClawUserDataDir(profile.name); - const profileState = getProfileState(); - - const httpReachable = await isHttpReachable(300); - if (httpReachable && !profileState.running) { - // Port in use but not by us - kill it - try { - const mod = await import("./pw-ai.js"); - await mod.closePlaywrightBrowserConnection(); - } catch { - // ignore - } - } - - if (profileState.running) { - await stopRunningBrowser(); - } - - try { - const mod = await import("./pw-ai.js"); - await mod.closePlaywrightBrowserConnection(); - } catch { - // ignore - } - - if (!fs.existsSync(userDataDir)) { - return { moved: false, from: userDataDir }; - } - - const moved = await movePathToTrash(userDataDir); - return { moved: true, from: userDataDir, to: moved }; - }; + const { resetProfile } = createProfileResetOps({ + profile, + getProfileState, + stopRunningBrowser, + isHttpReachable, + resolveOpenClawUserDataDir, + }); return { profile, diff --git a/src/slack/monitor/events/message-subtype-handlers.test.ts b/src/slack/monitor/events/message-subtype-handlers.test.ts new file mode 100644 index 00000000000..35923266b40 --- /dev/null +++ b/src/slack/monitor/events/message-subtype-handlers.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it } from "vitest"; +import type { SlackMessageEvent } from "../../types.js"; +import { resolveSlackMessageSubtypeHandler } from "./message-subtype-handlers.js"; + +describe("resolveSlackMessageSubtypeHandler", () => { + it("resolves message_changed metadata and identifiers", () => { + const event = { + type: "message", + subtype: "message_changed", + channel: "D1", + event_ts: "123.456", + message: { ts: "123.456", user: "U1" }, + previous_message: { ts: "123.450", user: "U2" }, + } as unknown as SlackMessageEvent; + + const handler = resolveSlackMessageSubtypeHandler(event); + expect(handler?.eventKind).toBe("message_changed"); + expect(handler?.resolveSenderId(event)).toBe("U1"); + expect(handler?.resolveChannelId(event)).toBe("D1"); + expect(handler?.resolveChannelType(event)).toBeUndefined(); + expect(handler?.contextKey(event)).toBe("slack:message:changed:D1:123.456"); + expect(handler?.describe("DM with @user")).toContain("edited"); + }); + + it("resolves message_deleted metadata and identifiers", () => { + const event = { + type: "message", + subtype: "message_deleted", + channel: "C1", + deleted_ts: "123.456", + event_ts: "123.457", + previous_message: { ts: "123.450", user: "U1" }, + } as unknown as SlackMessageEvent; + + const handler = resolveSlackMessageSubtypeHandler(event); + expect(handler?.eventKind).toBe("message_deleted"); + expect(handler?.resolveSenderId(event)).toBe("U1"); + expect(handler?.resolveChannelId(event)).toBe("C1"); + expect(handler?.resolveChannelType(event)).toBeUndefined(); + expect(handler?.contextKey(event)).toBe("slack:message:deleted:C1:123.456"); + expect(handler?.describe("general")).toContain("deleted"); + }); + + it("resolves thread_broadcast metadata and identifiers", () => { + const event = { + type: "message", + subtype: "thread_broadcast", + channel: "C1", + event_ts: "123.456", + message: { ts: "123.456", user: "U1" }, + user: "U1", + } as unknown as SlackMessageEvent; + + const handler = resolveSlackMessageSubtypeHandler(event); + expect(handler?.eventKind).toBe("thread_broadcast"); + expect(handler?.resolveSenderId(event)).toBe("U1"); + expect(handler?.resolveChannelId(event)).toBe("C1"); + expect(handler?.resolveChannelType(event)).toBeUndefined(); + expect(handler?.contextKey(event)).toBe("slack:thread:broadcast:C1:123.456"); + expect(handler?.describe("general")).toContain("broadcast"); + }); + + it("returns undefined for regular messages", () => { + const event = { + type: "message", + channel: "D1", + user: "U1", + text: "hello", + } as unknown as SlackMessageEvent; + expect(resolveSlackMessageSubtypeHandler(event)).toBeUndefined(); + }); +}); diff --git a/src/slack/monitor/events/message-subtype-handlers.ts b/src/slack/monitor/events/message-subtype-handlers.ts new file mode 100644 index 00000000000..524baf0cb67 --- /dev/null +++ b/src/slack/monitor/events/message-subtype-handlers.ts @@ -0,0 +1,98 @@ +import type { SlackMessageEvent } from "../../types.js"; +import type { + SlackMessageChangedEvent, + SlackMessageDeletedEvent, + SlackThreadBroadcastEvent, +} from "../types.js"; + +type SupportedSubtype = "message_changed" | "message_deleted" | "thread_broadcast"; + +export type SlackMessageSubtypeHandler = { + subtype: SupportedSubtype; + eventKind: SupportedSubtype; + describe: (channelLabel: string) => string; + contextKey: (event: SlackMessageEvent) => string; + resolveSenderId: (event: SlackMessageEvent) => string | undefined; + resolveChannelId: (event: SlackMessageEvent) => string | undefined; + resolveChannelType: (event: SlackMessageEvent) => string | null | undefined; +}; + +const changedHandler: SlackMessageSubtypeHandler = { + subtype: "message_changed", + eventKind: "message_changed", + describe: (channelLabel) => `Slack message edited in ${channelLabel}.`, + contextKey: (event) => { + const changed = event as SlackMessageChangedEvent; + const channelId = changed.channel ?? "unknown"; + const messageId = + changed.message?.ts ?? changed.previous_message?.ts ?? changed.event_ts ?? "unknown"; + return `slack:message:changed:${channelId}:${messageId}`; + }, + resolveSenderId: (event) => { + const changed = event as SlackMessageChangedEvent; + return ( + changed.message?.user ?? + changed.previous_message?.user ?? + changed.message?.bot_id ?? + changed.previous_message?.bot_id + ); + }, + resolveChannelId: (event) => (event as SlackMessageChangedEvent).channel, + resolveChannelType: () => undefined, +}; + +const deletedHandler: SlackMessageSubtypeHandler = { + subtype: "message_deleted", + eventKind: "message_deleted", + describe: (channelLabel) => `Slack message deleted in ${channelLabel}.`, + contextKey: (event) => { + const deleted = event as SlackMessageDeletedEvent; + const channelId = deleted.channel ?? "unknown"; + const messageId = deleted.deleted_ts ?? deleted.event_ts ?? "unknown"; + return `slack:message:deleted:${channelId}:${messageId}`; + }, + resolveSenderId: (event) => { + const deleted = event as SlackMessageDeletedEvent; + return deleted.previous_message?.user ?? deleted.previous_message?.bot_id; + }, + resolveChannelId: (event) => (event as SlackMessageDeletedEvent).channel, + resolveChannelType: () => undefined, +}; + +const threadBroadcastHandler: SlackMessageSubtypeHandler = { + subtype: "thread_broadcast", + eventKind: "thread_broadcast", + describe: (channelLabel) => `Slack thread reply broadcast in ${channelLabel}.`, + contextKey: (event) => { + const thread = event as SlackThreadBroadcastEvent; + const channelId = thread.channel ?? "unknown"; + const messageId = thread.message?.ts ?? thread.event_ts ?? "unknown"; + return `slack:thread:broadcast:${channelId}:${messageId}`; + }, + resolveSenderId: (event) => { + const thread = event as SlackThreadBroadcastEvent; + return thread.user ?? thread.message?.user ?? thread.message?.bot_id; + }, + resolveChannelId: (event) => (event as SlackThreadBroadcastEvent).channel, + resolveChannelType: () => undefined, +}; + +const SUBTYPE_HANDLER_REGISTRY: Record = { + message_changed: changedHandler, + message_deleted: deletedHandler, + thread_broadcast: threadBroadcastHandler, +}; + +export function resolveSlackMessageSubtypeHandler( + event: SlackMessageEvent, +): SlackMessageSubtypeHandler | undefined { + const subtype = event.subtype; + if ( + subtype !== "message_changed" && + subtype !== "message_deleted" && + subtype !== "thread_broadcast" + ) { + return undefined; + } + return SUBTYPE_HANDLER_REGISTRY[subtype]; +} diff --git a/src/slack/monitor/events/messages.ts b/src/slack/monitor/events/messages.ts index 6891b18c3b9..fac307416e4 100644 --- a/src/slack/monitor/events/messages.ts +++ b/src/slack/monitor/events/messages.ts @@ -4,11 +4,7 @@ import { enqueueSystemEvent } from "../../../infra/system-events.js"; import type { SlackAppMentionEvent, SlackMessageEvent } from "../../types.js"; import type { SlackMonitorContext } from "../context.js"; import type { SlackMessageHandler } from "../message-handler.js"; -import type { - SlackMessageChangedEvent, - SlackMessageDeletedEvent, - SlackThreadBroadcastEvent, -} from "../types.js"; +import { resolveSlackMessageSubtypeHandler } from "./message-subtype-handlers.js"; import { authorizeAndResolveSlackSystemEventContext } from "./system-event-context.js"; export function registerSlackMessageEvents(params: { @@ -17,16 +13,6 @@ export function registerSlackMessageEvents(params: { }) { const { ctx, handleSlackMessage } = params; - const resolveChangedSenderId = (changed: SlackMessageChangedEvent): string | undefined => - changed.message?.user ?? - changed.previous_message?.user ?? - changed.message?.bot_id ?? - changed.previous_message?.bot_id; - const resolveDeletedSenderId = (deleted: SlackMessageDeletedEvent): string | undefined => - deleted.previous_message?.user ?? deleted.previous_message?.bot_id; - const resolveThreadBroadcastSenderId = (thread: SlackThreadBroadcastEvent): string | undefined => - thread.user ?? thread.message?.user ?? thread.message?.bot_id; - const handleIncomingMessageEvent = async ({ event, body }: { event: unknown; body: unknown }) => { try { if (ctx.shouldDropMismatchedSlackEvent(body)) { @@ -34,59 +20,22 @@ export function registerSlackMessageEvents(params: { } const message = event as SlackMessageEvent; - if (message.subtype === "message_changed") { - const changed = event as SlackMessageChangedEvent; - const channelId = changed.channel; + const subtypeHandler = resolveSlackMessageSubtypeHandler(message); + if (subtypeHandler) { + const channelId = subtypeHandler.resolveChannelId(message); const ingressContext = await authorizeAndResolveSlackSystemEventContext({ ctx, - senderId: resolveChangedSenderId(changed), + senderId: subtypeHandler.resolveSenderId(message), channelId, - eventKind: "message_changed", + channelType: subtypeHandler.resolveChannelType(message), + eventKind: subtypeHandler.eventKind, }); if (!ingressContext) { return; } - const messageId = changed.message?.ts ?? changed.previous_message?.ts; - enqueueSystemEvent(`Slack message edited in ${ingressContext.channelLabel}.`, { + enqueueSystemEvent(subtypeHandler.describe(ingressContext.channelLabel), { sessionKey: ingressContext.sessionKey, - contextKey: `slack:message:changed:${channelId ?? "unknown"}:${messageId ?? changed.event_ts ?? "unknown"}`, - }); - return; - } - if (message.subtype === "message_deleted") { - const deleted = event as SlackMessageDeletedEvent; - const channelId = deleted.channel; - const ingressContext = await authorizeAndResolveSlackSystemEventContext({ - ctx, - senderId: resolveDeletedSenderId(deleted), - channelId, - eventKind: "message_deleted", - }); - if (!ingressContext) { - return; - } - enqueueSystemEvent(`Slack message deleted in ${ingressContext.channelLabel}.`, { - sessionKey: ingressContext.sessionKey, - contextKey: `slack:message:deleted:${channelId ?? "unknown"}:${deleted.deleted_ts ?? deleted.event_ts ?? "unknown"}`, - }); - return; - } - if (message.subtype === "thread_broadcast") { - const thread = event as SlackThreadBroadcastEvent; - const channelId = thread.channel; - const ingressContext = await authorizeAndResolveSlackSystemEventContext({ - ctx, - senderId: resolveThreadBroadcastSenderId(thread), - channelId, - eventKind: "thread_broadcast", - }); - if (!ingressContext) { - return; - } - const messageId = thread.message?.ts ?? thread.event_ts; - enqueueSystemEvent(`Slack thread reply broadcast in ${ingressContext.channelLabel}.`, { - sessionKey: ingressContext.sessionKey, - contextKey: `slack:thread:broadcast:${channelId ?? "unknown"}:${messageId ?? "unknown"}`, + contextKey: subtypeHandler.contextKey(message), }); return; }