From 067855e62319609622f514269167a04a2fb2e192 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 2 Mar 2026 21:29:10 +0000 Subject: [PATCH] refactor(browser): dedupe browser and cli command wiring --- src/browser/extension-relay-auth.test.ts | 47 +- ...er-context.remote-profile-tab-ops.suite.ts | 273 ++++++++ ...ver-context.remote-profile-tab-ops.test.ts | 274 +------- .../server-context.remote-tab-ops.test.ts | 625 +----------------- ...erver-context.tab-selection-state.suite.ts | 245 +++++++ ...server-context.tab-selection-state.test.ts | 256 +------ .../register.element.ts | 210 +++--- .../register.files-downloads.ts | 154 +++-- .../browser-cli-manage.timeout-option.test.ts | 12 +- src/cli/browser-cli-state.cookies-storage.ts | 152 ++--- ...rowser-cli-state.option-collisions.test.ts | 21 +- src/cli/browser-cli-test-helpers.ts | 19 + 12 files changed, 807 insertions(+), 1481 deletions(-) create mode 100644 src/browser/server-context.remote-profile-tab-ops.suite.ts create mode 100644 src/browser/server-context.tab-selection-state.suite.ts create mode 100644 src/cli/browser-cli-test-helpers.ts diff --git a/src/browser/extension-relay-auth.test.ts b/src/browser/extension-relay-auth.test.ts index 3410e1566..068f82b10 100644 --- a/src/browser/extension-relay-auth.test.ts +++ b/src/browser/extension-relay-auth.test.ts @@ -26,6 +26,23 @@ async function withRelayServer( } } +function handleNonVersionRequest(req: IncomingMessage, res: ServerResponse): boolean { + if (req.url?.startsWith("/json/version")) { + return false; + } + res.writeHead(404); + res.end("not found"); + return true; +} + +async function probeRelay(baseUrl: string, relayAuthToken: string): Promise { + return await probeAuthenticatedOpenClawRelay({ + baseUrl, + relayAuthHeader: "x-openclaw-relay-token", + relayAuthToken, + }); +} + describe("extension-relay-auth", () => { const TEST_GATEWAY_TOKEN = "test-gateway-token"; let prevGatewayToken: string | undefined; @@ -63,9 +80,7 @@ describe("extension-relay-auth", () => { let seenToken: string | undefined; await withRelayServer( (req, res) => { - if (!req.url?.startsWith("/json/version")) { - res.writeHead(404); - res.end("not found"); + if (handleNonVersionRequest(req, res)) { return; } const header = req.headers["x-openclaw-relay-token"]; @@ -75,11 +90,7 @@ describe("extension-relay-auth", () => { }, async ({ port }) => { const token = resolveRelayAuthTokenForPort(port); - const ok = await probeAuthenticatedOpenClawRelay({ - baseUrl: `http://127.0.0.1:${port}`, - relayAuthHeader: "x-openclaw-relay-token", - relayAuthToken: token, - }); + const ok = await probeRelay(`http://127.0.0.1:${port}`, token); expect(ok).toBe(true); expect(seenToken).toBe(token); }, @@ -89,20 +100,14 @@ describe("extension-relay-auth", () => { it("rejects unauthenticated probe responses", async () => { await withRelayServer( (req, res) => { - if (!req.url?.startsWith("/json/version")) { - res.writeHead(404); - res.end("not found"); + if (handleNonVersionRequest(req, res)) { return; } res.writeHead(401); res.end("Unauthorized"); }, async ({ port }) => { - const ok = await probeAuthenticatedOpenClawRelay({ - baseUrl: `http://127.0.0.1:${port}`, - relayAuthHeader: "x-openclaw-relay-token", - relayAuthToken: "irrelevant", - }); + const ok = await probeRelay(`http://127.0.0.1:${port}`, "irrelevant"); expect(ok).toBe(false); }, ); @@ -111,20 +116,14 @@ describe("extension-relay-auth", () => { it("rejects probe responses with wrong browser identity", async () => { await withRelayServer( (req, res) => { - if (!req.url?.startsWith("/json/version")) { - res.writeHead(404); - res.end("not found"); + if (handleNonVersionRequest(req, res)) { return; } res.writeHead(200, { "Content-Type": "application/json" }); res.end(JSON.stringify({ Browser: "FakeRelay" })); }, async ({ port }) => { - const ok = await probeAuthenticatedOpenClawRelay({ - baseUrl: `http://127.0.0.1:${port}`, - relayAuthHeader: "x-openclaw-relay-token", - relayAuthToken: "irrelevant", - }); + const ok = await probeRelay(`http://127.0.0.1:${port}`, "irrelevant"); expect(ok).toBe(false); }, ); diff --git a/src/browser/server-context.remote-profile-tab-ops.suite.ts b/src/browser/server-context.remote-profile-tab-ops.suite.ts new file mode 100644 index 000000000..746a8c87f --- /dev/null +++ b/src/browser/server-context.remote-profile-tab-ops.suite.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-profile-tab-ops.test.ts b/src/browser/server-context.remote-profile-tab-ops.test.ts index 746a8c87f..2d4b563e0 100644 --- a/src/browser/server-context.remote-profile-tab-ops.test.ts +++ b/src/browser/server-context.remote-profile-tab-ops.test.ts @@ -1,273 +1 @@ -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(); - }); -}); +import "./server-context.remote-profile-tab-ops.suite.js"; diff --git a/src/browser/server-context.remote-tab-ops.test.ts b/src/browser/server-context.remote-tab-ops.test.ts index b61274783..358ffd891 100644 --- a/src/browser/server-context.remote-tab-ops.test.ts +++ b/src/browser/server-context.remote-tab-ops.test.ts @@ -1,623 +1,2 @@ -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 createOpenclawManagedTab(id: string, index: number): JsonListEntry { - return { - id, - title: String(index), - url: `http://127.0.0.1:300${index}`, - webSocketDebuggerUrl: `ws://127.0.0.1/devtools/page/${id}`, - type: "page", - }; -} - -function createOpenclawManagedTabs(params?: { - includeNew?: boolean; - newFirst?: boolean; -}): JsonListEntry[] { - const oldTabs = Array.from({ length: 8 }, (_, idx) => - createOpenclawManagedTab(`OLD${idx + 1}`, idx + 1), - ); - if (params?.includeNew === false) { - return oldTabs; - } - const newTab = createOpenclawManagedTab("NEW", 9); - return params?.newFirst ? [newTab, ...oldTabs] : [...oldTabs, newTab]; -} - -function createOpenclawRouteHarness( - fetchMock: ReturnType, - params?: { attachOnly?: boolean; seedRunningProfile?: boolean }, -) { - global.fetch = withFetchPreconnect(fetchMock); - const state = makeState("openclaw"); - if (params?.attachOnly) { - state.resolved.attachOnly = true; - } - if (params?.seedRunningProfile ?? true) { - (state.profiles as Map).set("openclaw", { - profile: { name: "openclaw" }, - running: { pid: 1234, proc: { on: vi.fn() } }, - lastTargetId: null, - }); - } - const ctx = createBrowserRouteContext({ getState: () => state }); - return { state, openclaw: ctx.forProfile("openclaw") }; -} - -function createJsonOkResponse(payload: unknown): Response { - return { - ok: true, - json: async () => payload, - } as unknown as Response; -} - -function createManagedTabsFetchMock(params: { - existingTabs: JsonListEntry[]; - onClose?: (url: string) => Promise | Response; -}) { - return vi.fn(async (url: unknown) => { - const value = String(url); - if (value.includes("/json/list")) { - return createJsonOkResponse(params.existingTabs); - } - if (value.includes("/json/close/")) { - if (params.onClose) { - return params.onClose(value); - } - throw new Error(`unexpected fetch: ${value}`); - } - throw new Error(`unexpected fetch: ${value}`); - }); -} - -async function expectManagedOldestTabClose(fetchMock: ReturnType) { - await vi.waitFor(() => { - expect(fetchMock).toHaveBeenCalledWith( - expect.stringContaining("/json/close/OLD1"), - expect.any(Object), - ); - }); -} - -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 = createOpenclawManagedTabs(); - - const fetchMock = createManagedTabsFetchMock({ - existingTabs, - onClose: (value) => { - if (value.includes("/json/close/OLD1")) { - return createJsonOkResponse({}); - } - throw new Error(`unexpected fetch: ${value}`); - }, - }); - - const { openclaw } = createOpenclawRouteHarness(fetchMock); - - const opened = await openclaw.openTab("http://127.0.0.1:3009"); - expect(opened.targetId).toBe("NEW"); - await expectManagedOldestTabClose(fetchMock); - }); - - it("never closes the just-opened managed tab during cap cleanup", async () => { - vi.spyOn(cdpModule, "createTargetViaCdp").mockResolvedValue({ targetId: "NEW" }); - const existingTabs = createOpenclawManagedTabs({ newFirst: true }); - - const fetchMock = createManagedTabsFetchMock({ - existingTabs, - onClose: (value) => { - if (value.includes("/json/close/OLD1")) { - return createJsonOkResponse({}); - } - if (value.includes("/json/close/NEW")) { - throw new Error("cleanup must not close NEW"); - } - throw new Error(`unexpected fetch: ${value}`); - }, - }); - - const { openclaw } = createOpenclawRouteHarness(fetchMock); - - const opened = await openclaw.openTab("http://127.0.0.1:3009"); - expect(opened.targetId).toBe("NEW"); - await expectManagedOldestTabClose(fetchMock); - 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}`); - }); - - const { openclaw } = createOpenclawRouteHarness(fetchMock); - - 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 = createOpenclawManagedTabs(); - - const fetchMock = createManagedTabsFetchMock({ - existingTabs, - onClose: (_value) => { - throw new Error("should not close tabs in attachOnly mode"); - }, - }); - - const { openclaw } = createOpenclawRouteHarness(fetchMock, { - attachOnly: true, - seedRunningProfile: false, - }); - - 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 = createOpenclawManagedTabs(); - - const fetchMock = createManagedTabsFetchMock({ - existingTabs, - onClose: (value) => { - if (value.includes("/json/close/OLD1")) { - return new Promise(() => {}); - } - throw new Error(`unexpected fetch: ${value}`); - }, - }); - - const { openclaw } = createOpenclawRouteHarness(fetchMock); - - 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(); - }); -}); +import "./server-context.remote-profile-tab-ops.suite.js"; +import "./server-context.tab-selection-state.suite.js"; diff --git a/src/browser/server-context.tab-selection-state.suite.ts b/src/browser/server-context.tab-selection-state.suite.ts new file mode 100644 index 000000000..d9541d91a --- /dev/null +++ b/src/browser/server-context.tab-selection-state.suite.ts @@ -0,0 +1,245 @@ +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(); +}); + +function seedRunningProfileState( + state: ReturnType, + profileName = "openclaw", +): void { + (state.profiles as Map).set(profileName, { + profile: { name: profileName }, + running: { pid: 1234, proc: { on: vi.fn() } }, + lastTargetId: null, + }); +} + +async function expectOldManagedTabClose(fetchMock: ReturnType): Promise { + await vi.waitFor(() => { + expect(fetchMock).toHaveBeenCalledWith( + expect.stringContaining("/json/close/OLD1"), + expect.any(Object), + ); + }); +} + +function createOldTabCleanupFetchMock( + existingTabs: ReturnType, + params?: { rejectNewTabClose?: boolean }, +): ReturnType { + return 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 (params?.rejectNewTabClose && value.includes("/json/close/NEW")) { + throw new Error("cleanup must not close NEW"); + } + throw new Error(`unexpected fetch: ${value}`); + }); +} + +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 = createOldTabCleanupFetchMock(existingTabs); + + global.fetch = withFetchPreconnect(fetchMock); + const state = makeState("openclaw"); + seedRunningProfileState(state); + 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 expectOldManagedTabClose(fetchMock); + }); + + 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 = createOldTabCleanupFetchMock(existingTabs, { rejectNewTabClose: true }); + + global.fetch = withFetchPreconnect(fetchMock); + const state = makeState("openclaw"); + seedRunningProfileState(state); + 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 expectOldManagedTabClose(fetchMock); + 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"); + seedRunningProfileState(state); + 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"); + seedRunningProfileState(state); + 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.tab-selection-state.test.ts b/src/browser/server-context.tab-selection-state.test.ts index a0d602074..edf810682 100644 --- a/src/browser/server-context.tab-selection-state.test.ts +++ b/src/browser/server-context.tab-selection-state.test.ts @@ -1,255 +1 @@ -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(); - }); -}); +import "./server-context.tab-selection-state.suite.js"; diff --git a/src/cli/browser-cli-actions-input/register.element.ts b/src/cli/browser-cli-actions-input/register.element.ts index fc2070807..2b27c349f 100644 --- a/src/cli/browser-cli-actions-input/register.element.ts +++ b/src/cli/browser-cli-actions-input/register.element.ts @@ -13,6 +13,31 @@ export function registerBrowserElementCommands( browser: Command, parentOpts: (cmd: Command) => BrowserParentOpts, ) { + const runElementAction = async (params: { + cmd: Command; + body: Record; + successMessage: string | ((result: unknown) => string); + timeoutMs?: number; + }): Promise => { + const { parent, profile } = resolveBrowserActionContext(params.cmd, parentOpts); + try { + const result = await callBrowserAct({ + parent, + profile, + body: params.body, + timeoutMs: params.timeoutMs, + }); + const successMessage = + typeof params.successMessage === "function" + ? params.successMessage(result) + : params.successMessage; + logBrowserActionResult(parent, result, successMessage); + } catch (err) { + defaultRuntime.error(danger(String(err))); + defaultRuntime.exit(1); + } + }; + browser .command("click") .description("Click an element by ref from snapshot") @@ -22,7 +47,6 @@ export function registerBrowserElementCommands( .option("--button ", "Mouse button to use") .option("--modifiers ", "Comma-separated modifiers (Shift,Alt,Meta)") .action(async (ref: string | undefined, opts, cmd) => { - const { parent, profile } = resolveBrowserActionContext(cmd, parentOpts); const refValue = requireRef(ref); if (!refValue) { return; @@ -33,25 +57,22 @@ export function registerBrowserElementCommands( .map((v: string) => v.trim()) .filter(Boolean) : undefined; - try { - const result = await callBrowserAct<{ url?: string }>({ - parent, - profile, - body: { - kind: "click", - ref: refValue, - targetId: opts.targetId?.trim() || undefined, - doubleClick: Boolean(opts.double), - button: opts.button?.trim() || undefined, - modifiers, - }, - }); - const suffix = result.url ? ` on ${result.url}` : ""; - logBrowserActionResult(parent, result, `clicked ref ${refValue}${suffix}`); - } catch (err) { - defaultRuntime.error(danger(String(err))); - defaultRuntime.exit(1); - } + await runElementAction({ + cmd, + body: { + kind: "click", + ref: refValue, + targetId: opts.targetId?.trim() || undefined, + doubleClick: Boolean(opts.double), + button: opts.button?.trim() || undefined, + modifiers, + }, + successMessage: (result) => { + const url = (result as { url?: unknown }).url; + const suffix = typeof url === "string" && url ? ` on ${url}` : ""; + return `clicked ref ${refValue}${suffix}`; + }, + }); }); browser @@ -63,29 +84,22 @@ export function registerBrowserElementCommands( .option("--slowly", "Type slowly (human-like)", false) .option("--target-id ", "CDP target id (or unique prefix)") .action(async (ref: string | undefined, text: string, opts, cmd) => { - const { parent, profile } = resolveBrowserActionContext(cmd, parentOpts); const refValue = requireRef(ref); if (!refValue) { return; } - try { - const result = await callBrowserAct({ - parent, - profile, - body: { - kind: "type", - ref: refValue, - text, - submit: Boolean(opts.submit), - slowly: Boolean(opts.slowly), - targetId: opts.targetId?.trim() || undefined, - }, - }); - logBrowserActionResult(parent, result, `typed into ref ${refValue}`); - } catch (err) { - defaultRuntime.error(danger(String(err))); - defaultRuntime.exit(1); - } + await runElementAction({ + cmd, + body: { + kind: "type", + ref: refValue, + text, + submit: Boolean(opts.submit), + slowly: Boolean(opts.slowly), + targetId: opts.targetId?.trim() || undefined, + }, + successMessage: `typed into ref ${refValue}`, + }); }); browser @@ -94,18 +108,11 @@ export function registerBrowserElementCommands( .argument("", "Key to press (e.g. Enter)") .option("--target-id ", "CDP target id (or unique prefix)") .action(async (key: string, opts, cmd) => { - const { parent, profile } = resolveBrowserActionContext(cmd, parentOpts); - try { - const result = await callBrowserAct({ - parent, - profile, - body: { kind: "press", key, targetId: opts.targetId?.trim() || undefined }, - }); - logBrowserActionResult(parent, result, `pressed ${key}`); - } catch (err) { - defaultRuntime.error(danger(String(err))); - defaultRuntime.exit(1); - } + await runElementAction({ + cmd, + body: { kind: "press", key, targetId: opts.targetId?.trim() || undefined }, + successMessage: `pressed ${key}`, + }); }); browser @@ -114,18 +121,11 @@ export function registerBrowserElementCommands( .argument("", "Ref id from snapshot") .option("--target-id ", "CDP target id (or unique prefix)") .action(async (ref: string, opts, cmd) => { - const { parent, profile } = resolveBrowserActionContext(cmd, parentOpts); - try { - const result = await callBrowserAct({ - parent, - profile, - body: { kind: "hover", ref, targetId: opts.targetId?.trim() || undefined }, - }); - logBrowserActionResult(parent, result, `hovered ref ${ref}`); - } catch (err) { - defaultRuntime.error(danger(String(err))); - defaultRuntime.exit(1); - } + await runElementAction({ + cmd, + body: { kind: "hover", ref, targetId: opts.targetId?.trim() || undefined }, + successMessage: `hovered ref ${ref}`, + }); }); browser @@ -137,28 +137,22 @@ export function registerBrowserElementCommands( Number(v), ) .action(async (ref: string | undefined, opts, cmd) => { - const { parent, profile } = resolveBrowserActionContext(cmd, parentOpts); const refValue = requireRef(ref); if (!refValue) { return; } - try { - const result = await callBrowserAct({ - parent, - profile, - body: { - kind: "scrollIntoView", - ref: refValue, - targetId: opts.targetId?.trim() || undefined, - timeoutMs: Number.isFinite(opts.timeoutMs) ? opts.timeoutMs : undefined, - }, - timeoutMs: Number.isFinite(opts.timeoutMs) ? opts.timeoutMs : undefined, - }); - logBrowserActionResult(parent, result, `scrolled into view: ${refValue}`); - } catch (err) { - defaultRuntime.error(danger(String(err))); - defaultRuntime.exit(1); - } + const timeoutMs = Number.isFinite(opts.timeoutMs) ? opts.timeoutMs : undefined; + await runElementAction({ + cmd, + body: { + kind: "scrollIntoView", + ref: refValue, + targetId: opts.targetId?.trim() || undefined, + timeoutMs, + }, + timeoutMs, + successMessage: `scrolled into view: ${refValue}`, + }); }); browser @@ -168,23 +162,16 @@ export function registerBrowserElementCommands( .argument("", "End ref id") .option("--target-id ", "CDP target id (or unique prefix)") .action(async (startRef: string, endRef: string, opts, cmd) => { - const { parent, profile } = resolveBrowserActionContext(cmd, parentOpts); - try { - const result = await callBrowserAct({ - parent, - profile, - body: { - kind: "drag", - startRef, - endRef, - targetId: opts.targetId?.trim() || undefined, - }, - }); - logBrowserActionResult(parent, result, `dragged ${startRef} → ${endRef}`); - } catch (err) { - defaultRuntime.error(danger(String(err))); - defaultRuntime.exit(1); - } + await runElementAction({ + cmd, + body: { + kind: "drag", + startRef, + endRef, + targetId: opts.targetId?.trim() || undefined, + }, + successMessage: `dragged ${startRef} → ${endRef}`, + }); }); browser @@ -194,22 +181,15 @@ export function registerBrowserElementCommands( .argument("", "Option values to select") .option("--target-id ", "CDP target id (or unique prefix)") .action(async (ref: string, values: string[], opts, cmd) => { - const { parent, profile } = resolveBrowserActionContext(cmd, parentOpts); - try { - const result = await callBrowserAct({ - parent, - profile, - body: { - kind: "select", - ref, - values, - targetId: opts.targetId?.trim() || undefined, - }, - }); - logBrowserActionResult(parent, result, `selected ${values.join(", ")}`); - } catch (err) { - defaultRuntime.error(danger(String(err))); - defaultRuntime.exit(1); - } + await runElementAction({ + cmd, + body: { + kind: "select", + ref, + values, + targetId: opts.targetId?.trim() || undefined, + }, + successMessage: `selected ${values.join(", ")}`, + }); }); } diff --git a/src/cli/browser-cli-actions-input/register.files-downloads.ts b/src/cli/browser-cli-actions-input/register.files-downloads.ts index af12682e3..a818aee1f 100644 --- a/src/cli/browser-cli-actions-input/register.files-downloads.ts +++ b/src/cli/browser-cli-actions-input/register.files-downloads.ts @@ -18,6 +18,36 @@ async function normalizeUploadPaths(paths: string[]): Promise { return result.paths; } +async function runBrowserPostAction(params: { + parent: BrowserParentOpts; + profile: string | undefined; + path: string; + body: Record; + timeoutMs: number; + describeSuccess: (result: T) => string; +}): Promise { + try { + const result = await callBrowserRequest( + params.parent, + { + method: "POST", + path: params.path, + query: params.profile ? { profile: params.profile } : undefined, + body: params.body, + }, + { timeoutMs: params.timeoutMs }, + ); + if (params.parent?.json) { + defaultRuntime.log(JSON.stringify(result, null, 2)); + return; + } + defaultRuntime.log(params.describeSuccess(result)); + } catch (err) { + defaultRuntime.error(danger(String(err))); + defaultRuntime.exit(1); + } +} + export function registerBrowserFilesAndDownloadsCommands( browser: Command, parentOpts: (cmd: Command) => BrowserParentOpts, @@ -35,31 +65,19 @@ export function registerBrowserFilesAndDownloadsCommands( request: { path: string; body: Record }, ) => { const { parent, profile } = resolveBrowserActionContext(cmd, parentOpts); - try { - const { timeoutMs, targetId } = resolveTimeoutAndTarget(opts); - const result = await callBrowserRequest<{ download: { path: string } }>( - parent, - { - method: "POST", - path: request.path, - query: profile ? { profile } : undefined, - body: { - ...request.body, - targetId, - timeoutMs, - }, - }, - { timeoutMs: timeoutMs ?? 20000 }, - ); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(result, null, 2)); - return; - } - defaultRuntime.log(`downloaded: ${shortenHomePath(result.download.path)}`); - } catch (err) { - defaultRuntime.error(danger(String(err))); - defaultRuntime.exit(1); - } + const { timeoutMs, targetId } = resolveTimeoutAndTarget(opts); + await runBrowserPostAction<{ download: { path: string } }>({ + parent, + profile, + path: request.path, + body: { + ...request.body, + targetId, + timeoutMs, + }, + timeoutMs: timeoutMs ?? 20000, + describeSuccess: (result) => `downloaded: ${shortenHomePath(result.download.path)}`, + }); }; browser @@ -80,35 +98,23 @@ export function registerBrowserFilesAndDownloadsCommands( ) .action(async (paths: string[], opts, cmd) => { const { parent, profile } = resolveBrowserActionContext(cmd, parentOpts); - try { - const normalizedPaths = await normalizeUploadPaths(paths); - const { timeoutMs, targetId } = resolveTimeoutAndTarget(opts); - const result = await callBrowserRequest<{ download: { path: string } }>( - parent, - { - method: "POST", - path: "/hooks/file-chooser", - query: profile ? { profile } : undefined, - body: { - paths: normalizedPaths, - ref: opts.ref?.trim() || undefined, - inputRef: opts.inputRef?.trim() || undefined, - element: opts.element?.trim() || undefined, - targetId, - timeoutMs, - }, - }, - { timeoutMs: timeoutMs ?? 20000 }, - ); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(result, null, 2)); - return; - } - defaultRuntime.log(`upload armed for ${paths.length} file(s)`); - } catch (err) { - defaultRuntime.error(danger(String(err))); - defaultRuntime.exit(1); - } + const normalizedPaths = await normalizeUploadPaths(paths); + const { timeoutMs, targetId } = resolveTimeoutAndTarget(opts); + await runBrowserPostAction({ + parent, + profile, + path: "/hooks/file-chooser", + body: { + paths: normalizedPaths, + ref: opts.ref?.trim() || undefined, + inputRef: opts.inputRef?.trim() || undefined, + element: opts.element?.trim() || undefined, + targetId, + timeoutMs, + }, + timeoutMs: timeoutMs ?? 20000, + describeSuccess: () => `upload armed for ${paths.length} file(s)`, + }); }); browser @@ -177,31 +183,19 @@ export function registerBrowserFilesAndDownloadsCommands( defaultRuntime.exit(1); return; } - try { - const { timeoutMs, targetId } = resolveTimeoutAndTarget(opts); - const result = await callBrowserRequest( - parent, - { - method: "POST", - path: "/hooks/dialog", - query: profile ? { profile } : undefined, - body: { - accept, - promptText: opts.prompt?.trim() || undefined, - targetId, - timeoutMs, - }, - }, - { timeoutMs: timeoutMs ?? 20000 }, - ); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(result, null, 2)); - return; - } - defaultRuntime.log("dialog armed"); - } catch (err) { - defaultRuntime.error(danger(String(err))); - defaultRuntime.exit(1); - } + const { timeoutMs, targetId } = resolveTimeoutAndTarget(opts); + await runBrowserPostAction({ + parent, + profile, + path: "/hooks/dialog", + body: { + accept, + promptText: opts.prompt?.trim() || undefined, + targetId, + timeoutMs, + }, + timeoutMs: timeoutMs ?? 20000, + describeSuccess: () => "dialog armed", + }); }); } diff --git a/src/cli/browser-cli-manage.timeout-option.test.ts b/src/cli/browser-cli-manage.timeout-option.test.ts index 9b6347fd9..bb4d6469c 100644 --- a/src/cli/browser-cli-manage.timeout-option.test.ts +++ b/src/cli/browser-cli-manage.timeout-option.test.ts @@ -1,7 +1,6 @@ -import { Command } from "commander"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { registerBrowserManageCommands } from "./browser-cli-manage.js"; -import type { BrowserParentOpts } from "./browser-cli-shared.js"; +import { createBrowserProgram } from "./browser-cli-test-helpers.js"; const mocks = vi.hoisted(() => ({ callBrowserRequest: vi.fn(async (_opts: unknown, req: { path?: string }) => @@ -44,13 +43,8 @@ vi.mock("../runtime.js", () => ({ describe("browser manage start timeout option", () => { function createProgram() { - const program = new Command(); - const browser = program - .command("browser") - .option("--browser-profile ", "Browser profile") - .option("--json", "Output JSON", false) - .option("--timeout ", "Timeout in ms", "30000"); - const parentOpts = (cmd: Command) => cmd.parent?.opts?.() as BrowserParentOpts; + const { program, browser, parentOpts } = createBrowserProgram(); + browser.option("--timeout ", "Timeout in ms", "30000"); registerBrowserManageCommands(browser, parentOpts); return program; } diff --git a/src/cli/browser-cli-state.cookies-storage.ts b/src/cli/browser-cli-state.cookies-storage.ts index c3b03404f..01190b5b4 100644 --- a/src/cli/browser-cli-state.cookies-storage.ts +++ b/src/cli/browser-cli-state.cookies-storage.ts @@ -28,6 +28,24 @@ function resolveTargetId(rawTargetId: unknown, command: Command): string | undef return trimmed ? trimmed : undefined; } +async function runMutationRequest(params: { + parent: BrowserParentOpts; + request: Parameters[1]; + successMessage: string; +}) { + try { + const result = await callBrowserRequest(params.parent, params.request, { timeoutMs: 20000 }); + if (params.parent?.json) { + defaultRuntime.log(JSON.stringify(result, null, 2)); + return; + } + defaultRuntime.log(params.successMessage); + } catch (err) { + defaultRuntime.error(danger(String(err))); + defaultRuntime.exit(1); + } +} + export function registerBrowserCookiesAndStorageCommands( browser: Command, parentOpts: (cmd: Command) => BrowserParentOpts, @@ -81,29 +99,19 @@ export function registerBrowserCookiesAndStorageCommands( defaultRuntime.exit(1); return; } - try { - const result = await callBrowserRequest( - parent, - { - method: "POST", - path: "/cookies/set", - query: profile ? { profile } : undefined, - body: { - targetId, - cookie: { name, value, url }, - }, + await runMutationRequest({ + parent, + request: { + method: "POST", + path: "/cookies/set", + query: profile ? { profile } : undefined, + body: { + targetId, + cookie: { name, value, url }, }, - { timeoutMs: 20000 }, - ); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(result, null, 2)); - return; - } - defaultRuntime.log(`cookie set: ${name}`); - } catch (err) { - defaultRuntime.error(danger(String(err))); - defaultRuntime.exit(1); - } + }, + successMessage: `cookie set: ${name}`, + }); }); cookies @@ -114,28 +122,18 @@ export function registerBrowserCookiesAndStorageCommands( const parent = parentOpts(cmd); const profile = parent?.browserProfile; const targetId = resolveTargetId(opts.targetId, cmd); - try { - const result = await callBrowserRequest( - parent, - { - method: "POST", - path: "/cookies/clear", - query: profile ? { profile } : undefined, - body: { - targetId, - }, + await runMutationRequest({ + parent, + request: { + method: "POST", + path: "/cookies/clear", + query: profile ? { profile } : undefined, + body: { + targetId, }, - { timeoutMs: 20000 }, - ); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(result, null, 2)); - return; - } - defaultRuntime.log("cookies cleared"); - } catch (err) { - defaultRuntime.error(danger(String(err))); - defaultRuntime.exit(1); - } + }, + successMessage: "cookies cleared", + }); }); const storage = browser.command("storage").description("Read/write localStorage/sessionStorage"); @@ -187,30 +185,20 @@ export function registerBrowserCookiesAndStorageCommands( const parent = parentOpts(cmd2); const profile = parent?.browserProfile; const targetId = resolveTargetId(opts.targetId, cmd2); - try { - const result = await callBrowserRequest( - parent, - { - method: "POST", - path: `/storage/${kind}/set`, - query: profile ? { profile } : undefined, - body: { - key, - value, - targetId, - }, + await runMutationRequest({ + parent, + request: { + method: "POST", + path: `/storage/${kind}/set`, + query: profile ? { profile } : undefined, + body: { + key, + value, + targetId, }, - { timeoutMs: 20000 }, - ); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(result, null, 2)); - return; - } - defaultRuntime.log(`${kind}Storage set: ${key}`); - } catch (err) { - defaultRuntime.error(danger(String(err))); - defaultRuntime.exit(1); - } + }, + successMessage: `${kind}Storage set: ${key}`, + }); }); cmd @@ -221,28 +209,18 @@ export function registerBrowserCookiesAndStorageCommands( const parent = parentOpts(cmd2); const profile = parent?.browserProfile; const targetId = resolveTargetId(opts.targetId, cmd2); - try { - const result = await callBrowserRequest( - parent, - { - method: "POST", - path: `/storage/${kind}/clear`, - query: profile ? { profile } : undefined, - body: { - targetId, - }, + await runMutationRequest({ + parent, + request: { + method: "POST", + path: `/storage/${kind}/clear`, + query: profile ? { profile } : undefined, + body: { + targetId, }, - { timeoutMs: 20000 }, - ); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(result, null, 2)); - return; - } - defaultRuntime.log(`${kind}Storage cleared`); - } catch (err) { - defaultRuntime.error(danger(String(err))); - defaultRuntime.exit(1); - } + }, + successMessage: `${kind}Storage cleared`, + }); }); } diff --git a/src/cli/browser-cli-state.option-collisions.test.ts b/src/cli/browser-cli-state.option-collisions.test.ts index 917c6c455..2fb445c6a 100644 --- a/src/cli/browser-cli-state.option-collisions.test.ts +++ b/src/cli/browser-cli-state.option-collisions.test.ts @@ -1,7 +1,6 @@ -import { Command } from "commander"; import { beforeEach, describe, expect, it, vi } from "vitest"; -import type { BrowserParentOpts } from "./browser-cli-shared.js"; import { registerBrowserStateCommands } from "./browser-cli-state.js"; +import { createBrowserProgram as createBrowserProgramShared } from "./browser-cli-test-helpers.js"; const mocks = vi.hoisted(() => ({ callBrowserRequest: vi.fn(async (..._args: unknown[]) => ({ ok: true })), @@ -26,16 +25,8 @@ vi.mock("../runtime.js", () => ({ })); describe("browser state option collisions", () => { - const createBrowserProgram = ({ withGatewayUrl = false } = {}) => { - const program = new Command(); - const browser = program - .command("browser") - .option("--browser-profile ", "Browser profile") - .option("--json", "Output JSON", false); - if (withGatewayUrl) { - browser.option("--url ", "Gateway WebSocket URL"); - } - const parentOpts = (cmd: Command) => cmd.parent?.opts?.() as BrowserParentOpts; + const createStateProgram = ({ withGatewayUrl = false } = {}) => { + const { program, browser, parentOpts } = createBrowserProgramShared({ withGatewayUrl }); registerBrowserStateCommands(browser, parentOpts); return program; }; @@ -50,7 +41,7 @@ describe("browser state option collisions", () => { }; const runBrowserCommand = async (argv: string[]) => { - const program = createBrowserProgram(); + const program = createStateProgram(); await program.parseAsync(["browser", ...argv], { from: "user" }); }; @@ -83,7 +74,7 @@ describe("browser state option collisions", () => { }); it("resolves --url via parent when addGatewayClientOptions captures it", async () => { - const program = createBrowserProgram({ withGatewayUrl: true }); + const program = createStateProgram({ withGatewayUrl: true }); await program.parseAsync( [ "browser", @@ -105,7 +96,7 @@ describe("browser state option collisions", () => { }); it("inherits --url from parent when subcommand does not provide it", async () => { - const program = createBrowserProgram({ withGatewayUrl: true }); + const program = createStateProgram({ withGatewayUrl: true }); await program.parseAsync( ["browser", "--url", "https://inherited.example.com", "cookies", "set", "session", "abc"], { from: "user" }, diff --git a/src/cli/browser-cli-test-helpers.ts b/src/cli/browser-cli-test-helpers.ts new file mode 100644 index 000000000..012a78618 --- /dev/null +++ b/src/cli/browser-cli-test-helpers.ts @@ -0,0 +1,19 @@ +import { Command } from "commander"; +import type { BrowserParentOpts } from "./browser-cli-shared.js"; + +export function createBrowserProgram(params?: { withGatewayUrl?: boolean }): { + program: Command; + browser: Command; + parentOpts: (cmd: Command) => BrowserParentOpts; +} { + const program = new Command(); + const browser = program + .command("browser") + .option("--browser-profile ", "Browser profile") + .option("--json", "Output JSON", false); + if (params?.withGatewayUrl) { + browser.option("--url ", "Gateway WebSocket URL"); + } + const parentOpts = (cmd: Command) => cmd.parent?.opts?.() as BrowserParentOpts; + return { program, browser, parentOpts }; +}