diff --git a/CHANGELOG.md b/CHANGELOG.md index adbaceac95e..d5c395b4214 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,9 +10,13 @@ Docs: https://docs.openclaw.ai - Docker/timezone override: add `OPENCLAW_TZ` so `docker-setup.sh` can pin gateway and CLI containers to a chosen IANA timezone instead of inheriting the daemon default. (#34119) Thanks @Lanfei. - iOS/onboarding: add a first-run welcome pager before gateway setup, stop auto-opening the QR scanner, and show `/pair qr` instructions on the connect step. (#45054) Thanks @ngutman. - Browser/existing-session: add an official Chrome DevTools MCP attach mode for signed-in live Chrome sessions, with docs for `chrome://inspect/#remote-debugging` enablement and direct backlinks to Chrome’s own setup guides. +- Browser/act automation: add batched actions, selector targeting, and delayed clicks for browser act requests with normalized batch dispatch. Thanks @vincentkoc. ### Fixes +- Dashboard/agents and chat: scope the Agents > Skills view to the selected agent, restore clean sidebar status and log rendering, and keep oversized inline markdown images from expanding across the chat UI. (#45451) Thanks @BunsDev. +- Telegram/webhook auth: validate the Telegram webhook secret before reading or parsing request bodies, so unauthenticated requests are rejected immediately instead of consuming up to 1 MB first. Thanks @space08. +- Build/plugin-sdk bundling: bundle plugin-sdk subpath entries in one shared build pass so published packages stop duplicating shared chunks and avoid the recent plugin-sdk memory blow-up. (#45426) Thanks @TarasShyn. - Dashboard/agents and chat: scope the Agents > Skills view to the selected agent, restore clean sidebar status and log rendering, and keep oversized inline markdown images from expanding across the chat UI. (#45451) Thanks @BunsDev. - Browser/existing-session: accept text-only `list_pages` and `new_page` responses from Chrome DevTools MCP so live-session tab discovery and new-tab open flows keep working when the server omits structured page metadata. - Ollama/reasoning visibility: stop promoting native `thinking` and `reasoning` fields into final assistant text so local reasoning models no longer leak internal thoughts in normal replies. (#45330) Thanks @xi7ang. diff --git a/docs/platforms/android.md b/docs/platforms/android.md index 4df71b83e73..6bd5effb361 100644 --- a/docs/platforms/android.md +++ b/docs/platforms/android.md @@ -9,6 +9,8 @@ title: "Android App" # Android App (Node) +> **Note:** The Android app has not been publicly released yet. The source code is available in the [OpenClaw repository](https://github.com/openclaw/openclaw) under `apps/android`. You can build it yourself using Java 17 and the Android SDK (`./gradlew :app:assembleDebug`). See [apps/android/README.md](https://github.com/openclaw/openclaw/blob/main/apps/android/README.md) for build instructions. + ## Support snapshot - Role: companion node app (Android does not host the Gateway). diff --git a/extensions/bluebubbles/src/media-send.test.ts b/extensions/bluebubbles/src/media-send.test.ts index 9f065599bfb..59fe82cbeae 100644 --- a/extensions/bluebubbles/src/media-send.test.ts +++ b/extensions/bluebubbles/src/media-send.test.ts @@ -70,6 +70,70 @@ async function makeTempDir(): Promise { return dir; } +async function makeTempFile( + fileName: string, + contents: string, + dir?: string, +): Promise<{ dir: string; filePath: string }> { + const resolvedDir = dir ?? (await makeTempDir()); + const filePath = path.join(resolvedDir, fileName); + await fs.writeFile(filePath, contents, "utf8"); + return { dir: resolvedDir, filePath }; +} + +async function sendLocalMedia(params: { + cfg: OpenClawConfig; + mediaPath: string; + accountId?: string; +}) { + return sendBlueBubblesMedia({ + cfg: params.cfg, + to: "chat:123", + accountId: params.accountId, + mediaPath: params.mediaPath, + }); +} + +async function expectRejectedLocalMedia(params: { + cfg: OpenClawConfig; + mediaPath: string; + error: RegExp; + accountId?: string; +}) { + await expect( + sendLocalMedia({ + cfg: params.cfg, + mediaPath: params.mediaPath, + accountId: params.accountId, + }), + ).rejects.toThrow(params.error); + + expect(sendBlueBubblesAttachmentMock).not.toHaveBeenCalled(); +} + +async function expectAllowedLocalMedia(params: { + cfg: OpenClawConfig; + mediaPath: string; + expectedAttachment: Record; + accountId?: string; + expectMimeDetection?: boolean; +}) { + const result = await sendLocalMedia({ + cfg: params.cfg, + mediaPath: params.mediaPath, + accountId: params.accountId, + }); + + expect(result).toEqual({ messageId: "msg-1" }); + expect(sendBlueBubblesAttachmentMock).toHaveBeenCalledTimes(1); + expect(sendBlueBubblesAttachmentMock.mock.calls[0]?.[0]).toEqual( + expect.objectContaining(params.expectedAttachment), + ); + if (params.expectMimeDetection) { + expect(runtimeMocks.detectMime).toHaveBeenCalled(); + } +} + beforeEach(() => { const runtime = createMockRuntime(); runtimeMocks = runtime.mocks; @@ -110,57 +174,43 @@ describe("sendBlueBubblesMedia local-path hardening", () => { const outsideFile = path.join(outsideDir, "outside.txt"); await fs.writeFile(outsideFile, "not allowed", "utf8"); - await expect( - sendBlueBubblesMedia({ - cfg: createConfig({ mediaLocalRoots: [allowedRoot] }), - to: "chat:123", - mediaPath: outsideFile, - }), - ).rejects.toThrow(/not under any configured mediaLocalRoots/i); - - expect(sendBlueBubblesAttachmentMock).not.toHaveBeenCalled(); + await expectRejectedLocalMedia({ + cfg: createConfig({ mediaLocalRoots: [allowedRoot] }), + mediaPath: outsideFile, + error: /not under any configured mediaLocalRoots/i, + }); }); it("allows local paths that are explicitly configured", async () => { - const allowedRoot = await makeTempDir(); - const allowedFile = path.join(allowedRoot, "allowed.txt"); - await fs.writeFile(allowedFile, "allowed", "utf8"); + const { dir: allowedRoot, filePath: allowedFile } = await makeTempFile( + "allowed.txt", + "allowed", + ); - const result = await sendBlueBubblesMedia({ + await expectAllowedLocalMedia({ cfg: createConfig({ mediaLocalRoots: [allowedRoot] }), - to: "chat:123", mediaPath: allowedFile, - }); - - expect(result).toEqual({ messageId: "msg-1" }); - expect(sendBlueBubblesAttachmentMock).toHaveBeenCalledTimes(1); - expect(sendBlueBubblesAttachmentMock.mock.calls[0]?.[0]).toEqual( - expect.objectContaining({ + expectedAttachment: { filename: "allowed.txt", contentType: "text/plain", - }), - ); - expect(runtimeMocks.detectMime).toHaveBeenCalled(); + }, + expectMimeDetection: true, + }); }); it("allows file:// media paths and file:// local roots", async () => { - const allowedRoot = await makeTempDir(); - const allowedFile = path.join(allowedRoot, "allowed.txt"); - await fs.writeFile(allowedFile, "allowed", "utf8"); - - const result = await sendBlueBubblesMedia({ - cfg: createConfig({ mediaLocalRoots: [pathToFileURL(allowedRoot).toString()] }), - to: "chat:123", - mediaPath: pathToFileURL(allowedFile).toString(), - }); - - expect(result).toEqual({ messageId: "msg-1" }); - expect(sendBlueBubblesAttachmentMock).toHaveBeenCalledTimes(1); - expect(sendBlueBubblesAttachmentMock.mock.calls[0]?.[0]).toEqual( - expect.objectContaining({ - filename: "allowed.txt", - }), + const { dir: allowedRoot, filePath: allowedFile } = await makeTempFile( + "allowed.txt", + "allowed", ); + + await expectAllowedLocalMedia({ + cfg: createConfig({ mediaLocalRoots: [pathToFileURL(allowedRoot).toString()] }), + mediaPath: pathToFileURL(allowedFile).toString(), + expectedAttachment: { + filename: "allowed.txt", + }, + }); }); it("uses account-specific mediaLocalRoots over top-level roots", async () => { @@ -213,15 +263,11 @@ describe("sendBlueBubblesMedia local-path hardening", () => { return; } - await expect( - sendBlueBubblesMedia({ - cfg: createConfig({ mediaLocalRoots: [allowedRoot] }), - to: "chat:123", - mediaPath: linkPath, - }), - ).rejects.toThrow(/not under any configured mediaLocalRoots/i); - - expect(sendBlueBubblesAttachmentMock).not.toHaveBeenCalled(); + await expectRejectedLocalMedia({ + cfg: createConfig({ mediaLocalRoots: [allowedRoot] }), + mediaPath: linkPath, + error: /not under any configured mediaLocalRoots/i, + }); }); it("rejects relative mediaLocalRoots entries", async () => { diff --git a/extensions/bluebubbles/src/monitor.webhook-auth.test.ts b/extensions/bluebubbles/src/monitor.webhook-auth.test.ts index 3685142508e..29b23a7001e 100644 --- a/extensions/bluebubbles/src/monitor.webhook-auth.test.ts +++ b/extensions/bluebubbles/src/monitor.webhook-auth.test.ts @@ -331,12 +331,13 @@ describe("BlueBubbles webhook monitor", () => { const req = new EventEmitter() as IncomingMessage & { destroy: (error?: Error) => IncomingMessage; }; + const destroyMock = vi.fn(); req.method = "POST"; req.url = url; req.headers = {}; - req.destroy = vi.fn((_: Error | undefined) => req) as typeof req.destroy; + req.destroy = destroyMock as unknown as typeof req.destroy; setRequestRemoteAddress(req, "127.0.0.1"); - return req; + return { req, destroyMock }; } function registerWebhookTargets( @@ -417,7 +418,7 @@ describe("BlueBubbles webhook monitor", () => { setupWebhookTarget(); // Create a request that never sends data or ends (simulates slow-loris) - const req = createHangingWebhookRequest(); + const { req, destroyMock } = createHangingWebhookRequest(); const res = createMockResponse(); @@ -429,7 +430,7 @@ describe("BlueBubbles webhook monitor", () => { const handled = await handledPromise; expect(handled).toBe(true); expect(res.statusCode).toBe(408); - expect(req.destroy).toHaveBeenCalled(); + expect(destroyMock).toHaveBeenCalled(); } finally { vi.useRealTimers(); } @@ -438,7 +439,7 @@ describe("BlueBubbles webhook monitor", () => { it("rejects unauthorized requests before reading the body", async () => { const account = createMockAccount({ password: "secret-token" }); setupWebhookTarget({ account }); - const req = createHangingWebhookRequest("/bluebubbles-webhook?password=wrong-token"); + const { req } = createHangingWebhookRequest("/bluebubbles-webhook?password=wrong-token"); const onSpy = vi.spyOn(req, "on"); await expectWebhookStatus(req, 401); expect(onSpy).not.toHaveBeenCalledWith("data", expect.any(Function)); diff --git a/extensions/feishu/src/probe.test.ts b/extensions/feishu/src/probe.test.ts index b93935cccc6..bfc270a4459 100644 --- a/extensions/feishu/src/probe.test.ts +++ b/extensions/feishu/src/probe.test.ts @@ -8,6 +8,22 @@ vi.mock("./client.js", () => ({ import { FEISHU_PROBE_REQUEST_TIMEOUT_MS, probeFeishu, clearProbeCache } from "./probe.js"; +const DEFAULT_CREDS = { appId: "cli_123", appSecret: "secret" } as const; // pragma: allowlist secret +const DEFAULT_SUCCESS_RESPONSE = { + code: 0, + bot: { bot_name: "TestBot", open_id: "ou_abc123" }, +} as const; +const DEFAULT_SUCCESS_RESULT = { + ok: true, + appId: "cli_123", + botName: "TestBot", + botOpenId: "ou_abc123", +} as const; +const BOT1_RESPONSE = { + code: 0, + bot: { bot_name: "Bot1", open_id: "ou_1" }, +} as const; + function makeRequestFn(response: Record) { return vi.fn().mockResolvedValue(response); } @@ -18,6 +34,64 @@ function setupClient(response: Record) { return requestFn; } +function setupSuccessClient() { + return setupClient(DEFAULT_SUCCESS_RESPONSE); +} + +async function expectDefaultSuccessResult( + creds = DEFAULT_CREDS, + expected: Awaited> = DEFAULT_SUCCESS_RESULT, +) { + const result = await probeFeishu(creds); + expect(result).toEqual(expected); +} + +async function withFakeTimers(run: () => Promise) { + vi.useFakeTimers(); + try { + await run(); + } finally { + vi.useRealTimers(); + } +} + +async function expectErrorResultCached(params: { + requestFn: ReturnType; + expectedError: string; + ttlMs: number; +}) { + createFeishuClientMock.mockReturnValue({ request: params.requestFn }); + + const first = await probeFeishu(DEFAULT_CREDS); + const second = await probeFeishu(DEFAULT_CREDS); + expect(first).toMatchObject({ ok: false, error: params.expectedError }); + expect(second).toMatchObject({ ok: false, error: params.expectedError }); + expect(params.requestFn).toHaveBeenCalledTimes(1); + + vi.advanceTimersByTime(params.ttlMs + 1); + + await probeFeishu(DEFAULT_CREDS); + expect(params.requestFn).toHaveBeenCalledTimes(2); +} + +async function expectFreshDefaultProbeAfter( + requestFn: ReturnType, + invalidate: () => void, +) { + await probeFeishu(DEFAULT_CREDS); + expect(requestFn).toHaveBeenCalledTimes(1); + + invalidate(); + + await probeFeishu(DEFAULT_CREDS); + expect(requestFn).toHaveBeenCalledTimes(2); +} + +async function readSequentialDefaultProbePair() { + const first = await probeFeishu(DEFAULT_CREDS); + return { first, second: await probeFeishu(DEFAULT_CREDS) }; +} + describe("probeFeishu", () => { beforeEach(() => { clearProbeCache(); @@ -44,28 +118,16 @@ describe("probeFeishu", () => { }); it("returns bot info on successful probe", async () => { - const requestFn = setupClient({ - code: 0, - bot: { bot_name: "TestBot", open_id: "ou_abc123" }, - }); + const requestFn = setupSuccessClient(); - const result = await probeFeishu({ appId: "cli_123", appSecret: "secret" }); // pragma: allowlist secret - expect(result).toEqual({ - ok: true, - appId: "cli_123", - botName: "TestBot", - botOpenId: "ou_abc123", - }); + await expectDefaultSuccessResult(); expect(requestFn).toHaveBeenCalledTimes(1); }); it("passes the probe timeout to the Feishu request", async () => { - const requestFn = setupClient({ - code: 0, - bot: { bot_name: "TestBot", open_id: "ou_abc123" }, - }); + const requestFn = setupSuccessClient(); - await probeFeishu({ appId: "cli_123", appSecret: "secret" }); // pragma: allowlist secret + await probeFeishu(DEFAULT_CREDS); expect(requestFn).toHaveBeenCalledWith( expect.objectContaining({ @@ -77,19 +139,16 @@ describe("probeFeishu", () => { }); it("returns timeout error when request exceeds timeout", async () => { - vi.useFakeTimers(); - try { + await withFakeTimers(async () => { const requestFn = vi.fn().mockImplementation(() => new Promise(() => {})); createFeishuClientMock.mockReturnValue({ request: requestFn }); - const promise = probeFeishu({ appId: "cli_123", appSecret: "secret" }, { timeoutMs: 1_000 }); + const promise = probeFeishu(DEFAULT_CREDS, { timeoutMs: 1_000 }); await vi.advanceTimersByTimeAsync(1_000); const result = await promise; expect(result).toMatchObject({ ok: false, error: "probe timed out after 1000ms" }); - } finally { - vi.useRealTimers(); - } + }); }); it("returns aborted when abort signal is already aborted", async () => { @@ -106,14 +165,9 @@ describe("probeFeishu", () => { expect(createFeishuClientMock).not.toHaveBeenCalled(); }); it("returns cached result on subsequent calls within TTL", async () => { - const requestFn = setupClient({ - code: 0, - bot: { bot_name: "TestBot", open_id: "ou_abc123" }, - }); + const requestFn = setupSuccessClient(); - const creds = { appId: "cli_123", appSecret: "secret" }; // pragma: allowlist secret - const first = await probeFeishu(creds); - const second = await probeFeishu(creds); + const { first, second } = await readSequentialDefaultProbePair(); expect(first).toEqual(second); // Only one API call should have been made @@ -121,76 +175,37 @@ describe("probeFeishu", () => { }); it("makes a fresh API call after cache expires", async () => { - vi.useFakeTimers(); - try { - const requestFn = setupClient({ - code: 0, - bot: { bot_name: "TestBot", open_id: "ou_abc123" }, + await withFakeTimers(async () => { + const requestFn = setupSuccessClient(); + + await expectFreshDefaultProbeAfter(requestFn, () => { + vi.advanceTimersByTime(10 * 60 * 1000 + 1); }); - - const creds = { appId: "cli_123", appSecret: "secret" }; // pragma: allowlist secret - await probeFeishu(creds); - expect(requestFn).toHaveBeenCalledTimes(1); - - // Advance time past the success TTL - vi.advanceTimersByTime(10 * 60 * 1000 + 1); - - await probeFeishu(creds); - expect(requestFn).toHaveBeenCalledTimes(2); - } finally { - vi.useRealTimers(); - } + }); }); it("caches failed probe results (API error) for the error TTL", async () => { - vi.useFakeTimers(); - try { - const requestFn = makeRequestFn({ code: 99, msg: "token expired" }); - createFeishuClientMock.mockReturnValue({ request: requestFn }); - - const creds = { appId: "cli_123", appSecret: "secret" }; // pragma: allowlist secret - const first = await probeFeishu(creds); - const second = await probeFeishu(creds); - expect(first).toMatchObject({ ok: false, error: "API error: token expired" }); - expect(second).toMatchObject({ ok: false, error: "API error: token expired" }); - expect(requestFn).toHaveBeenCalledTimes(1); - - vi.advanceTimersByTime(60 * 1000 + 1); - - await probeFeishu(creds); - expect(requestFn).toHaveBeenCalledTimes(2); - } finally { - vi.useRealTimers(); - } + await withFakeTimers(async () => { + await expectErrorResultCached({ + requestFn: makeRequestFn({ code: 99, msg: "token expired" }), + expectedError: "API error: token expired", + ttlMs: 60 * 1000, + }); + }); }); it("caches thrown request errors for the error TTL", async () => { - vi.useFakeTimers(); - try { - const requestFn = vi.fn().mockRejectedValue(new Error("network error")); - createFeishuClientMock.mockReturnValue({ request: requestFn }); - - const creds = { appId: "cli_123", appSecret: "secret" }; // pragma: allowlist secret - const first = await probeFeishu(creds); - const second = await probeFeishu(creds); - expect(first).toMatchObject({ ok: false, error: "network error" }); - expect(second).toMatchObject({ ok: false, error: "network error" }); - expect(requestFn).toHaveBeenCalledTimes(1); - - vi.advanceTimersByTime(60 * 1000 + 1); - - await probeFeishu(creds); - expect(requestFn).toHaveBeenCalledTimes(2); - } finally { - vi.useRealTimers(); - } + await withFakeTimers(async () => { + await expectErrorResultCached({ + requestFn: vi.fn().mockRejectedValue(new Error("network error")), + expectedError: "network error", + ttlMs: 60 * 1000, + }); + }); }); it("caches per account independently", async () => { - const requestFn = setupClient({ - code: 0, - bot: { bot_name: "Bot1", open_id: "ou_1" }, - }); + const requestFn = setupClient(BOT1_RESPONSE); await probeFeishu({ appId: "cli_aaa", appSecret: "s1" }); // pragma: allowlist secret expect(requestFn).toHaveBeenCalledTimes(1); @@ -205,10 +220,7 @@ describe("probeFeishu", () => { }); it("does not share cache between accounts with same appId but different appSecret", async () => { - const requestFn = setupClient({ - code: 0, - bot: { bot_name: "Bot1", open_id: "ou_1" }, - }); + const requestFn = setupClient(BOT1_RESPONSE); // First account with appId + secret A await probeFeishu({ appId: "cli_shared", appSecret: "secret_aaa" }); // pragma: allowlist secret @@ -221,10 +233,7 @@ describe("probeFeishu", () => { }); it("uses accountId for cache key when available", async () => { - const requestFn = setupClient({ - code: 0, - bot: { bot_name: "Bot1", open_id: "ou_1" }, - }); + const requestFn = setupClient(BOT1_RESPONSE); // Two accounts with same appId+appSecret but different accountIds are cached separately await probeFeishu({ accountId: "acct-1", appId: "cli_123", appSecret: "secret" }); // pragma: allowlist secret @@ -239,19 +248,11 @@ describe("probeFeishu", () => { }); it("clearProbeCache forces fresh API call", async () => { - const requestFn = setupClient({ - code: 0, - bot: { bot_name: "TestBot", open_id: "ou_abc123" }, + const requestFn = setupSuccessClient(); + + await expectFreshDefaultProbeAfter(requestFn, () => { + clearProbeCache(); }); - - const creds = { appId: "cli_123", appSecret: "secret" }; // pragma: allowlist secret - await probeFeishu(creds); - expect(requestFn).toHaveBeenCalledTimes(1); - - clearProbeCache(); - - await probeFeishu(creds); - expect(requestFn).toHaveBeenCalledTimes(2); }); it("handles response.data.bot fallback path", async () => { @@ -260,10 +261,8 @@ describe("probeFeishu", () => { data: { bot: { bot_name: "DataBot", open_id: "ou_data" } }, }); - const result = await probeFeishu({ appId: "cli_123", appSecret: "secret" }); // pragma: allowlist secret - expect(result).toEqual({ - ok: true, - appId: "cli_123", + await expectDefaultSuccessResult(DEFAULT_CREDS, { + ...DEFAULT_SUCCESS_RESULT, botName: "DataBot", botOpenId: "ou_data", }); diff --git a/extensions/matrix/src/matrix/monitor/events.test.ts b/extensions/matrix/src/matrix/monitor/events.test.ts index 603344d323f..6dac0db59fc 100644 --- a/extensions/matrix/src/matrix/monitor/events.test.ts +++ b/extensions/matrix/src/matrix/monitor/events.test.ts @@ -14,15 +14,15 @@ vi.mock("../send.js", () => ({ describe("registerMatrixMonitorEvents", () => { const roomId = "!room:example.org"; - function createRoomEvent(event: Partial): MatrixRawEvent { + function makeEvent(overrides: Partial): MatrixRawEvent { return { - event_id: "$default", - sender: "@bot:example.org", + event_id: "$event", + sender: "@alice:example.org", type: "m.room.message", origin_server_ts: 0, content: {}, - ...event, - } as MatrixRawEvent; + ...overrides, + }; } beforeEach(() => { @@ -78,7 +78,7 @@ describe("registerMatrixMonitorEvents", () => { it("sends read receipt immediately for non-self messages", async () => { const { client, onRoomMessage, roomMessageHandler } = createHarness(); - const event = createRoomEvent({ + const event = makeEvent({ event_id: "$e1", sender: "@alice:example.org", }); @@ -93,7 +93,7 @@ describe("registerMatrixMonitorEvents", () => { it("does not send read receipts for self messages", async () => { await expectForwardedWithoutReadReceipt( - createRoomEvent({ + makeEvent({ event_id: "$e2", sender: "@bot:example.org", }), @@ -102,16 +102,17 @@ describe("registerMatrixMonitorEvents", () => { it("skips receipt when message lacks sender or event id", async () => { await expectForwardedWithoutReadReceipt( - createRoomEvent({ + makeEvent({ sender: "@alice:example.org", + event_id: "", }), ); }); it("caches self user id across messages", async () => { const { getUserId, roomMessageHandler } = createHarness(); - const first = createRoomEvent({ event_id: "$e3", sender: "@alice:example.org" }); - const second = createRoomEvent({ event_id: "$e4", sender: "@bob:example.org" }); + const first = makeEvent({ event_id: "$e3", sender: "@alice:example.org" }); + const second = makeEvent({ event_id: "$e4", sender: "@bob:example.org" }); roomMessageHandler("!room:example.org", first); roomMessageHandler("!room:example.org", second); @@ -125,7 +126,7 @@ describe("registerMatrixMonitorEvents", () => { it("logs and continues when sending read receipt fails", async () => { sendReadReceiptMatrixMock.mockRejectedValueOnce(new Error("network boom")); const { roomMessageHandler, onRoomMessage, logVerboseMessage } = createHarness(); - const event = createRoomEvent({ event_id: "$e5", sender: "@alice:example.org" }); + const event = makeEvent({ event_id: "$e5", sender: "@alice:example.org" }); roomMessageHandler("!room:example.org", event); @@ -141,7 +142,7 @@ describe("registerMatrixMonitorEvents", () => { const { roomMessageHandler, onRoomMessage, getUserId } = createHarness({ getUserId: vi.fn().mockRejectedValue(new Error("cannot resolve self")), }); - const event = createRoomEvent({ event_id: "$e6", sender: "@alice:example.org" }); + const event = makeEvent({ event_id: "$e6", sender: "@alice:example.org" }); roomMessageHandler("!room:example.org", event); diff --git a/extensions/msteams/src/monitor-handler.file-consent.test.ts b/extensions/msteams/src/monitor-handler.file-consent.test.ts index 88a6a67a838..5e72f7a9dd1 100644 --- a/extensions/msteams/src/monitor-handler.file-consent.test.ts +++ b/extensions/msteams/src/monitor-handler.file-consent.test.ts @@ -123,6 +123,26 @@ function createInvokeContext(params: { }; } +function createConsentInvokeHarness(params: { + pendingConversationId?: string; + invokeConversationId: string; + action: "accept" | "decline"; +}) { + const uploadId = storePendingUpload({ + buffer: Buffer.from("TOP_SECRET_VICTIM_FILE\n"), + filename: "secret.txt", + contentType: "text/plain", + conversationId: params.pendingConversationId ?? "19:victim@thread.v2", + }); + const handler = registerMSTeamsHandlers(createActivityHandler(), createDeps()); + const { context, sendActivity } = createInvokeContext({ + conversationId: params.invokeConversationId, + uploadId, + action: params.action, + }); + return { uploadId, handler, context, sendActivity }; +} + describe("msteams file consent invoke authz", () => { beforeEach(() => { setMSTeamsRuntime(runtimeStub); @@ -132,17 +152,8 @@ describe("msteams file consent invoke authz", () => { }); it("uploads when invoke conversation matches pending upload conversation", async () => { - const uploadId = storePendingUpload({ - buffer: Buffer.from("TOP_SECRET_VICTIM_FILE\n"), - filename: "secret.txt", - contentType: "text/plain", - conversationId: "19:victim@thread.v2", - }); - const deps = createDeps(); - const handler = registerMSTeamsHandlers(createActivityHandler(), deps); - const { context, sendActivity } = createInvokeContext({ - conversationId: "19:victim@thread.v2;messageid=abc123", - uploadId, + const { uploadId, handler, context, sendActivity } = createConsentInvokeHarness({ + invokeConversationId: "19:victim@thread.v2;messageid=abc123", action: "accept", }); @@ -166,17 +177,8 @@ describe("msteams file consent invoke authz", () => { }); it("rejects cross-conversation accept invoke and keeps pending upload", async () => { - const uploadId = storePendingUpload({ - buffer: Buffer.from("TOP_SECRET_VICTIM_FILE\n"), - filename: "secret.txt", - contentType: "text/plain", - conversationId: "19:victim@thread.v2", - }); - const deps = createDeps(); - const handler = registerMSTeamsHandlers(createActivityHandler(), deps); - const { context, sendActivity } = createInvokeContext({ - conversationId: "19:attacker@thread.v2", - uploadId, + const { uploadId, handler, context, sendActivity } = createConsentInvokeHarness({ + invokeConversationId: "19:attacker@thread.v2", action: "accept", }); @@ -198,17 +200,8 @@ describe("msteams file consent invoke authz", () => { }); it("ignores cross-conversation decline invoke and keeps pending upload", async () => { - const uploadId = storePendingUpload({ - buffer: Buffer.from("TOP_SECRET_VICTIM_FILE\n"), - filename: "secret.txt", - contentType: "text/plain", - conversationId: "19:victim@thread.v2", - }); - const deps = createDeps(); - const handler = registerMSTeamsHandlers(createActivityHandler(), deps); - const { context, sendActivity } = createInvokeContext({ - conversationId: "19:attacker@thread.v2", - uploadId, + const { uploadId, handler, context, sendActivity } = createConsentInvokeHarness({ + invokeConversationId: "19:attacker@thread.v2", action: "decline", }); diff --git a/extensions/msteams/src/policy.test.ts b/extensions/msteams/src/policy.test.ts index 091e22d1fd8..ac324f3d785 100644 --- a/extensions/msteams/src/policy.test.ts +++ b/extensions/msteams/src/policy.test.ts @@ -6,6 +6,27 @@ import { resolveMSTeamsRouteConfig, } from "./policy.js"; +function resolveNamedTeamRouteConfig(allowNameMatching = false) { + const cfg: MSTeamsConfig = { + teams: { + "My Team": { + requireMention: true, + channels: { + "General Chat": { requireMention: false }, + }, + }, + }, + }; + + return resolveMSTeamsRouteConfig({ + cfg, + teamName: "My Team", + channelName: "General Chat", + conversationId: "ignored", + allowNameMatching, + }); +} + describe("msteams policy", () => { describe("resolveMSTeamsRouteConfig", () => { it("returns team and channel config when present", () => { @@ -51,23 +72,7 @@ describe("msteams policy", () => { }); it("blocks team and channel name matches by default", () => { - const cfg: MSTeamsConfig = { - teams: { - "My Team": { - requireMention: true, - channels: { - "General Chat": { requireMention: false }, - }, - }, - }, - }; - - const res = resolveMSTeamsRouteConfig({ - cfg, - teamName: "My Team", - channelName: "General Chat", - conversationId: "ignored", - }); + const res = resolveNamedTeamRouteConfig(); expect(res.teamConfig).toBeUndefined(); expect(res.channelConfig).toBeUndefined(); @@ -75,24 +80,7 @@ describe("msteams policy", () => { }); it("matches team and channel by name when dangerous name matching is enabled", () => { - const cfg: MSTeamsConfig = { - teams: { - "My Team": { - requireMention: true, - channels: { - "General Chat": { requireMention: false }, - }, - }, - }, - }; - - const res = resolveMSTeamsRouteConfig({ - cfg, - teamName: "My Team", - channelName: "General Chat", - conversationId: "ignored", - allowNameMatching: true, - }); + const res = resolveNamedTeamRouteConfig(true); expect(res.teamConfig?.requireMention).toBe(true); expect(res.channelConfig?.requireMention).toBe(false); diff --git a/extensions/nostr/src/nostr-profile-http.test.ts b/extensions/nostr/src/nostr-profile-http.test.ts index 8fb17c443f4..3caa739c6c1 100644 --- a/extensions/nostr/src/nostr-profile-http.test.ts +++ b/extensions/nostr/src/nostr-profile-http.test.ts @@ -115,6 +115,13 @@ function createMockContext(overrides?: Partial): NostrP }; } +function expectOkResponse(res: ReturnType) { + expect(res._getStatusCode()).toBe(200); + const data = JSON.parse(res._getData()); + expect(data.ok).toBe(true); + return data; +} + function mockSuccessfulProfileImport() { vi.mocked(importProfileFromRelays).mockResolvedValue({ ok: true, @@ -208,6 +215,22 @@ describe("nostr-profile-http", () => { }); describe("PUT /api/channels/nostr/:accountId/profile", () => { + function mockPublishSuccess() { + vi.mocked(publishNostrProfile).mockResolvedValue({ + eventId: "event123", + createdAt: 1234567890, + successes: ["wss://relay.damus.io"], + failures: [], + }); + } + + function expectBadRequestResponse(res: ReturnType) { + expect(res._getStatusCode()).toBe(400); + const data = JSON.parse(res._getData()); + expect(data.ok).toBe(false); + return data; + } + async function expectPrivatePictureRejected(pictureUrl: string) { const ctx = createMockContext(); const handler = createNostrProfileHttpHandler(ctx); @@ -219,9 +242,7 @@ describe("nostr-profile-http", () => { await handler(req, res); - expect(res._getStatusCode()).toBe(400); - const data = JSON.parse(res._getData()); - expect(data.ok).toBe(false); + const data = expectBadRequestResponse(res); expect(data.error).toContain("private"); } @@ -235,18 +256,11 @@ describe("nostr-profile-http", () => { }); const res = createMockResponse(); - vi.mocked(publishNostrProfile).mockResolvedValue({ - eventId: "event123", - createdAt: 1234567890, - successes: ["wss://relay.damus.io"], - failures: [], - }); + mockPublishSuccess(); await handler(req, res); - expect(res._getStatusCode()).toBe(200); - const data = JSON.parse(res._getData()); - expect(data.ok).toBe(true); + const data = expectOkResponse(res); expect(data.eventId).toBe("event123"); expect(data.successes).toContain("wss://relay.damus.io"); expect(data.persisted).toBe(true); @@ -332,9 +346,7 @@ describe("nostr-profile-http", () => { await handler(req, res); - expect(res._getStatusCode()).toBe(400); - const data = JSON.parse(res._getData()); - expect(data.ok).toBe(false); + const data = expectBadRequestResponse(res); // The schema validation catches non-https URLs before SSRF check expect(data.error).toBe("Validation failed"); expect(data.details).toBeDefined(); @@ -368,12 +380,7 @@ describe("nostr-profile-http", () => { const ctx = createMockContext(); const handler = createNostrProfileHttpHandler(ctx); - vi.mocked(publishNostrProfile).mockResolvedValue({ - eventId: "event123", - createdAt: 1234567890, - successes: ["wss://relay.damus.io"], - failures: [], - }); + mockPublishSuccess(); // Make 6 requests (limit is 5/min) for (let i = 0; i < 6; i++) { @@ -384,7 +391,7 @@ describe("nostr-profile-http", () => { await handler(req, res); if (i < 5) { - expect(res._getStatusCode()).toBe(200); + expectOkResponse(res); } else { expect(res._getStatusCode()).toBe(429); const data = JSON.parse(res._getData()); @@ -414,6 +421,12 @@ describe("nostr-profile-http", () => { }); describe("POST /api/channels/nostr/:accountId/profile/import", () => { + function expectImportSuccessResponse(res: ReturnType) { + const data = expectOkResponse(res); + expect(data.imported.name).toBe("imported"); + return data; + } + it("imports profile from relays", async () => { const ctx = createMockContext(); const handler = createNostrProfileHttpHandler(ctx); @@ -424,10 +437,7 @@ describe("nostr-profile-http", () => { await handler(req, res); - expect(res._getStatusCode()).toBe(200); - const data = JSON.parse(res._getData()); - expect(data.ok).toBe(true); - expect(data.imported.name).toBe("imported"); + const data = expectImportSuccessResponse(res); expect(data.saved).toBe(false); // autoMerge not requested }); @@ -490,8 +500,7 @@ describe("nostr-profile-http", () => { await handler(req, res); - expect(res._getStatusCode()).toBe(200); - const data = JSON.parse(res._getData()); + const data = expectImportSuccessResponse(res); expect(data.saved).toBe(true); expect(ctx.updateConfigProfile).toHaveBeenCalled(); }); diff --git a/extensions/slack/src/channel.ts b/extensions/slack/src/channel.ts index bd2b640c510..73c844a1cc0 100644 --- a/extensions/slack/src/channel.ts +++ b/extensions/slack/src/channel.ts @@ -244,6 +244,18 @@ export const slackPlugin: ChannelPlugin = { }, resolver: { resolveTargets: async ({ cfg, accountId, inputs, kind }) => { + const toResolvedTarget = < + T extends { input: string; resolved: boolean; id?: string; name?: string }, + >( + entry: T, + note?: string, + ) => ({ + input: entry.input, + resolved: entry.resolved, + id: entry.id, + name: entry.name, + note, + }); const account = resolveSlackAccount({ cfg, accountId }); const token = account.config.userToken?.trim() || account.botToken?.trim(); if (!token) { @@ -258,25 +270,15 @@ export const slackPlugin: ChannelPlugin = { token, entries: inputs, }); - return resolved.map((entry) => ({ - input: entry.input, - resolved: entry.resolved, - id: entry.id, - name: entry.name, - note: entry.archived ? "archived" : undefined, - })); + return resolved.map((entry) => + toResolvedTarget(entry, entry.archived ? "archived" : undefined), + ); } const resolved = await getSlackRuntime().channel.slack.resolveUserAllowlist({ token, entries: inputs, }); - return resolved.map((entry) => ({ - input: entry.input, - resolved: entry.resolved, - id: entry.id, - name: entry.name, - note: entry.note, - })); + return resolved.map((entry) => toResolvedTarget(entry, entry.note)); }, }, actions: { diff --git a/extensions/synology-chat/src/channel.test.ts b/extensions/synology-chat/src/channel.test.ts index 4e3be192f39..2814d437c6b 100644 --- a/extensions/synology-chat/src/channel.test.ts +++ b/extensions/synology-chat/src/channel.test.ts @@ -46,6 +46,23 @@ vi.mock("zod", () => ({ const { createSynologyChatPlugin } = await import("./channel.js"); const { registerPluginHttpRoute } = await import("openclaw/plugin-sdk/synology-chat"); +function makeSecurityAccount(overrides: Record = {}) { + return { + accountId: "default", + enabled: true, + token: "t", + incomingUrl: "https://nas/incoming", + nasHost: "h", + webhookPath: "/w", + dmPolicy: "allowlist" as const, + allowedUserIds: [], + rateLimitPerMinute: 30, + botName: "Bot", + allowInsecureSsl: false, + ...overrides, + }; +} + describe("createSynologyChatPlugin", () => { it("returns a plugin object with all required sections", () => { const plugin = createSynologyChatPlugin(); @@ -133,95 +150,35 @@ describe("createSynologyChatPlugin", () => { describe("security.collectWarnings", () => { it("warns when token is missing", () => { const plugin = createSynologyChatPlugin(); - const account = { - accountId: "default", - enabled: true, - token: "", - incomingUrl: "https://nas/incoming", - nasHost: "h", - webhookPath: "/w", - dmPolicy: "allowlist" as const, - allowedUserIds: [], - rateLimitPerMinute: 30, - botName: "Bot", - allowInsecureSsl: false, - }; + const account = makeSecurityAccount({ token: "" }); const warnings = plugin.security.collectWarnings({ account }); expect(warnings.some((w: string) => w.includes("token"))).toBe(true); }); it("warns when allowInsecureSsl is true", () => { const plugin = createSynologyChatPlugin(); - const account = { - accountId: "default", - enabled: true, - token: "t", - incomingUrl: "https://nas/incoming", - nasHost: "h", - webhookPath: "/w", - dmPolicy: "allowlist" as const, - allowedUserIds: [], - rateLimitPerMinute: 30, - botName: "Bot", - allowInsecureSsl: true, - }; + const account = makeSecurityAccount({ allowInsecureSsl: true }); const warnings = plugin.security.collectWarnings({ account }); expect(warnings.some((w: string) => w.includes("SSL"))).toBe(true); }); it("warns when dmPolicy is open", () => { const plugin = createSynologyChatPlugin(); - const account = { - accountId: "default", - enabled: true, - token: "t", - incomingUrl: "https://nas/incoming", - nasHost: "h", - webhookPath: "/w", - dmPolicy: "open" as const, - allowedUserIds: [], - rateLimitPerMinute: 30, - botName: "Bot", - allowInsecureSsl: false, - }; + const account = makeSecurityAccount({ dmPolicy: "open" }); const warnings = plugin.security.collectWarnings({ account }); expect(warnings.some((w: string) => w.includes("open"))).toBe(true); }); it("warns when dmPolicy is allowlist and allowedUserIds is empty", () => { const plugin = createSynologyChatPlugin(); - const account = { - accountId: "default", - enabled: true, - token: "t", - incomingUrl: "https://nas/incoming", - nasHost: "h", - webhookPath: "/w", - dmPolicy: "allowlist" as const, - allowedUserIds: [], - rateLimitPerMinute: 30, - botName: "Bot", - allowInsecureSsl: false, - }; + const account = makeSecurityAccount(); const warnings = plugin.security.collectWarnings({ account }); expect(warnings.some((w: string) => w.includes("empty allowedUserIds"))).toBe(true); }); it("returns no warnings for fully configured account", () => { const plugin = createSynologyChatPlugin(); - const account = { - accountId: "default", - enabled: true, - token: "t", - incomingUrl: "https://nas/incoming", - nasHost: "h", - webhookPath: "/w", - dmPolicy: "allowlist" as const, - allowedUserIds: ["user1"], - rateLimitPerMinute: 30, - botName: "Bot", - allowInsecureSsl: false, - }; + const account = makeSecurityAccount({ allowedUserIds: ["user1"] }); const warnings = plugin.security.collectWarnings({ account }); expect(warnings).toHaveLength(0); }); @@ -317,6 +274,23 @@ describe("createSynologyChatPlugin", () => { }); describe("gateway", () => { + function makeStartAccountCtx( + accountConfig: Record, + abortController = new AbortController(), + ) { + return { + abortController, + ctx: { + cfg: { + channels: { "synology-chat": accountConfig }, + }, + accountId: "default", + log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() }, + abortSignal: abortController.signal, + }, + }; + } + async function expectPendingStartAccountPromise( result: Promise, abortController: AbortController, @@ -333,15 +307,7 @@ describe("createSynologyChatPlugin", () => { async function expectPendingStartAccount(accountConfig: Record) { const plugin = createSynologyChatPlugin(); - const abortController = new AbortController(); - const ctx = { - cfg: { - channels: { "synology-chat": accountConfig }, - }, - accountId: "default", - log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() }, - abortSignal: abortController.signal, - }; + const { ctx, abortController } = makeStartAccountCtx(accountConfig); const result = plugin.gateway.startAccount(ctx); await expectPendingStartAccountPromise(result, abortController); } @@ -357,25 +323,14 @@ describe("createSynologyChatPlugin", () => { it("startAccount refuses allowlist accounts with empty allowedUserIds", async () => { const registerMock = vi.mocked(registerPluginHttpRoute); registerMock.mockClear(); - const abortController = new AbortController(); - const plugin = createSynologyChatPlugin(); - const ctx = { - cfg: { - channels: { - "synology-chat": { - enabled: true, - token: "t", - incomingUrl: "https://nas/incoming", - dmPolicy: "allowlist", - allowedUserIds: [], - }, - }, - }, - accountId: "default", - log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() }, - abortSignal: abortController.signal, - }; + const { ctx, abortController } = makeStartAccountCtx({ + enabled: true, + token: "t", + incomingUrl: "https://nas/incoming", + dmPolicy: "allowlist", + allowedUserIds: [], + }); const result = plugin.gateway.startAccount(ctx); await expectPendingStartAccountPromise(result, abortController); diff --git a/extensions/synology-chat/src/client.test.ts b/extensions/synology-chat/src/client.test.ts index 416412f0408..2ae24f42904 100644 --- a/extensions/synology-chat/src/client.test.ts +++ b/extensions/synology-chat/src/client.test.ts @@ -51,7 +51,7 @@ function mockFailureResponse(statusCode = 500) { mockResponse(statusCode, "error"); } -describe("sendMessage", () => { +function installFakeTimerHarness() { beforeEach(() => { vi.clearAllMocks(); vi.useFakeTimers(); @@ -62,6 +62,10 @@ describe("sendMessage", () => { afterEach(() => { vi.useRealTimers(); }); +} + +describe("sendMessage", () => { + installFakeTimerHarness(); it("returns true on successful send", async () => { mockSuccessResponse(); @@ -86,16 +90,7 @@ describe("sendMessage", () => { }); describe("sendFileUrl", () => { - beforeEach(() => { - vi.clearAllMocks(); - vi.useFakeTimers(); - fakeNowMs += 10_000; - vi.setSystemTime(fakeNowMs); - }); - - afterEach(() => { - vi.useRealTimers(); - }); + installFakeTimerHarness(); it("returns true on success", async () => { mockSuccessResponse(); diff --git a/extensions/synology-chat/src/client.ts b/extensions/synology-chat/src/client.ts index 95240e556f5..d66f1b720f4 100644 --- a/extensions/synology-chat/src/client.ts +++ b/extensions/synology-chat/src/client.ts @@ -27,6 +27,12 @@ type ChatUserCacheEntry = { cachedAt: number; }; +type ChatWebhookPayload = { + text?: string; + file_url?: string; + user_ids?: number[]; +}; + // Cache user lists per bot endpoint to avoid cross-account bleed. const chatUserCache = new Map(); const CACHE_TTL_MS = 5 * 60 * 1000; // 5 minutes @@ -47,16 +53,7 @@ export async function sendMessage( ): Promise { // Synology Chat API requires user_ids (numeric) to specify the recipient // The @mention is optional but user_ids is mandatory - const payloadObj: Record = { text }; - if (userId) { - // userId can be numeric ID or username - if numeric, add to user_ids - const numericId = typeof userId === "number" ? userId : parseInt(userId, 10); - if (!isNaN(numericId)) { - payloadObj.user_ids = [numericId]; - } - } - const payload = JSON.stringify(payloadObj); - const body = `payload=${encodeURIComponent(payload)}`; + const body = buildWebhookBody({ text }, userId); // Internal rate limit: min 500ms between sends const now = Date.now(); @@ -95,15 +92,7 @@ export async function sendFileUrl( userId?: string | number, allowInsecureSsl = true, ): Promise { - const payloadObj: Record = { file_url: fileUrl }; - if (userId) { - const numericId = typeof userId === "number" ? userId : parseInt(userId, 10); - if (!isNaN(numericId)) { - payloadObj.user_ids = [numericId]; - } - } - const payload = JSON.stringify(payloadObj); - const body = `payload=${encodeURIComponent(payload)}`; + const body = buildWebhookBody({ file_url: fileUrl }, userId); try { const ok = await doPost(incomingUrl, body, allowInsecureSsl); @@ -215,6 +204,22 @@ export async function resolveChatUserId( return undefined; } +function buildWebhookBody(payload: ChatWebhookPayload, userId?: string | number): string { + const numericId = parseNumericUserId(userId); + if (numericId !== undefined) { + payload.user_ids = [numericId]; + } + return `payload=${encodeURIComponent(JSON.stringify(payload))}`; +} + +function parseNumericUserId(userId?: string | number): number | undefined { + if (userId === undefined) { + return undefined; + } + const numericId = typeof userId === "number" ? userId : parseInt(userId, 10); + return Number.isNaN(numericId) ? undefined : numericId; +} + function doPost(url: string, body: string, allowInsecureSsl = true): Promise { return new Promise((resolve, reject) => { let parsedUrl: URL; diff --git a/extensions/synology-chat/src/webhook-handler.test.ts b/extensions/synology-chat/src/webhook-handler.test.ts index 37ee566e6a6..ae5bd061b85 100644 --- a/extensions/synology-chat/src/webhook-handler.test.ts +++ b/extensions/synology-chat/src/webhook-handler.test.ts @@ -2,6 +2,7 @@ import { EventEmitter } from "node:events"; import type { IncomingMessage, ServerResponse } from "node:http"; import { describe, it, expect, vi, beforeEach } from "vitest"; import type { ResolvedSynologyChatAccount } from "./types.js"; +import type { WebhookHandlerDeps } from "./webhook-handler.js"; import { clearSynologyWebhookRateLimiterStateForTest, createWebhookHandler, @@ -37,21 +38,7 @@ function makeReq( body: string, opts: { headers?: Record; url?: string } = {}, ): IncomingMessage { - const req = new EventEmitter() as IncomingMessage & { - destroyed: boolean; - }; - req.method = method; - req.headers = opts.headers ?? {}; - req.url = opts.url ?? "/webhook/synology"; - req.socket = { remoteAddress: "127.0.0.1" } as any; - req.destroyed = false; - req.destroy = ((_: Error | undefined) => { - if (req.destroyed) { - return req; - } - req.destroyed = true; - return req; - }) as IncomingMessage["destroy"]; + const req = makeBaseReq(method, opts); // Simulate body delivery process.nextTick(() => { @@ -65,11 +52,19 @@ function makeReq( return req; } function makeStalledReq(method: string): IncomingMessage { + return makeBaseReq(method); +} + +function makeBaseReq( + method: string, + opts: { headers?: Record; url?: string } = {}, +): IncomingMessage & { destroyed: boolean } { const req = new EventEmitter() as IncomingMessage & { destroyed: boolean; }; req.method = method; - req.headers = {}; + req.headers = opts.headers ?? {}; + req.url = opts.url ?? "/webhook/synology"; req.socket = { remoteAddress: "127.0.0.1" } as any; req.destroyed = false; req.destroy = ((_: Error | undefined) => { @@ -124,10 +119,12 @@ describe("createWebhookHandler", () => { async function expectForbiddenByPolicy(params: { account: Partial; bodyContains: string; + deliver?: WebhookHandlerDeps["deliver"]; }) { + const deliver = params.deliver ?? vi.fn(); const handler = createWebhookHandler({ account: makeAccount(params.account), - deliver: vi.fn(), + deliver, log, }); @@ -137,6 +134,7 @@ describe("createWebhookHandler", () => { expect(res._status).toBe(403); expect(res._body).toContain(params.bodyContains); + expect(deliver).not.toHaveBeenCalled(); } it("rejects non-POST methods with 405", async () => { @@ -302,22 +300,14 @@ describe("createWebhookHandler", () => { it("returns 403 when allowlist policy is set with empty allowedUserIds", async () => { const deliver = vi.fn(); - const handler = createWebhookHandler({ - account: makeAccount({ + await expectForbiddenByPolicy({ + account: { dmPolicy: "allowlist", allowedUserIds: [], - }), + }, + bodyContains: "Allowlist is empty", deliver, - log, }); - - const req = makeReq("POST", validBody); - const res = makeRes(); - await handler(req, res); - - expect(res._status).toBe(403); - expect(res._body).toContain("Allowlist is empty"); - expect(deliver).not.toHaveBeenCalled(); }); it("returns 403 when DMs are disabled", async () => { diff --git a/extensions/twitch/src/access-control.test.ts b/extensions/twitch/src/access-control.test.ts index 874326c9697..3d522246700 100644 --- a/extensions/twitch/src/access-control.test.ts +++ b/extensions/twitch/src/access-control.test.ts @@ -49,6 +49,41 @@ describe("checkTwitchAccessControl", () => { return result; } + function expectAllowedAccessCheck(params: { + account?: Partial; + message?: Partial; + }) { + const result = runAccessCheck({ + account: params.account, + message: { + message: "@testbot hello", + ...params.message, + }, + }); + expect(result.allowed).toBe(true); + return result; + } + + function expectAllowFromBlocked(params: { + allowFrom: string[]; + allowedRoles?: NonNullable; + message?: Partial; + reason: string; + }) { + const result = runAccessCheck({ + account: { + allowFrom: params.allowFrom, + allowedRoles: params.allowedRoles, + }, + message: { + message: "@testbot hello", + ...params.message, + }, + }); + expect(result.allowed).toBe(false); + expect(result.reason).toContain(params.reason); + } + describe("when no restrictions are configured", () => { it("allows messages that mention the bot (default requireMention)", () => { const result = runAccessCheck({ @@ -109,62 +144,28 @@ describe("checkTwitchAccessControl", () => { describe("allowFrom allowlist", () => { it("allows users in the allowlist", () => { - const account: TwitchAccountConfig = { - ...mockAccount, - allowFrom: ["123456", "789012"], - }; - const message: TwitchChatMessage = { - ...mockMessage, - message: "@testbot hello", - }; - - const result = checkTwitchAccessControl({ - message, - account, - botUsername: "testbot", + const result = expectAllowedAccessCheck({ + account: { + allowFrom: ["123456", "789012"], + }, }); - expect(result.allowed).toBe(true); expect(result.matchKey).toBe("123456"); expect(result.matchSource).toBe("allowlist"); }); it("blocks users not in allowlist when allowFrom is set", () => { - const account: TwitchAccountConfig = { - ...mockAccount, + expectAllowFromBlocked({ allowFrom: ["789012"], - }; - const message: TwitchChatMessage = { - ...mockMessage, - message: "@testbot hello", - }; - - const result = checkTwitchAccessControl({ - message, - account, - botUsername: "testbot", + reason: "allowFrom", }); - expect(result.allowed).toBe(false); - expect(result.reason).toContain("allowFrom"); }); it("blocks messages without userId", () => { - const account: TwitchAccountConfig = { - ...mockAccount, + expectAllowFromBlocked({ allowFrom: ["123456"], - }; - const message: TwitchChatMessage = { - ...mockMessage, - message: "@testbot hello", - userId: undefined, - }; - - const result = checkTwitchAccessControl({ - message, - account, - botUsername: "testbot", + message: { userId: undefined }, + reason: "user ID not available", }); - expect(result.allowed).toBe(false); - expect(result.reason).toContain("user ID not available"); }); it("bypasses role checks when user is in allowlist", () => { @@ -188,47 +189,21 @@ describe("checkTwitchAccessControl", () => { }); it("blocks user with role when not in allowlist", () => { - const account: TwitchAccountConfig = { - ...mockAccount, + expectAllowFromBlocked({ allowFrom: ["789012"], allowedRoles: ["moderator"], - }; - const message: TwitchChatMessage = { - ...mockMessage, - message: "@testbot hello", - userId: "123456", - isMod: true, - }; - - const result = checkTwitchAccessControl({ - message, - account, - botUsername: "testbot", + message: { userId: "123456", isMod: true }, + reason: "allowFrom", }); - expect(result.allowed).toBe(false); - expect(result.reason).toContain("allowFrom"); }); it("blocks user not in allowlist even when roles configured", () => { - const account: TwitchAccountConfig = { - ...mockAccount, + expectAllowFromBlocked({ allowFrom: ["789012"], allowedRoles: ["moderator"], - }; - const message: TwitchChatMessage = { - ...mockMessage, - message: "@testbot hello", - userId: "123456", - isMod: false, - }; - - const result = checkTwitchAccessControl({ - message, - account, - botUsername: "testbot", + message: { userId: "123456", isMod: false }, + reason: "allowFrom", }); - expect(result.allowed).toBe(false); - expect(result.reason).toContain("allowFrom"); }); }); @@ -283,21 +258,11 @@ describe("checkTwitchAccessControl", () => { }); it("allows all users when role is 'all'", () => { - const account: TwitchAccountConfig = { - ...mockAccount, - allowedRoles: ["all"], - }; - const message: TwitchChatMessage = { - ...mockMessage, - message: "@testbot hello", - }; - - const result = checkTwitchAccessControl({ - message, - account, - botUsername: "testbot", + const result = expectAllowedAccessCheck({ + account: { + allowedRoles: ["all"], + }, }); - expect(result.allowed).toBe(true); expect(result.matchKey).toBe("all"); }); diff --git a/extensions/zalo/src/monitor.ts b/extensions/zalo/src/monitor.ts index bd1351bd147..d82c0d96ba4 100644 --- a/extensions/zalo/src/monitor.ts +++ b/extensions/zalo/src/monitor.ts @@ -75,6 +75,35 @@ const WEBHOOK_CLEANUP_TIMEOUT_MS = 5_000; const ZALO_TYPING_TIMEOUT_MS = 5_000; type ZaloCoreRuntime = ReturnType; +type ZaloStatusSink = (patch: { lastInboundAt?: number; lastOutboundAt?: number }) => void; +type ZaloProcessingContext = { + token: string; + account: ResolvedZaloAccount; + config: OpenClawConfig; + runtime: ZaloRuntimeEnv; + core: ZaloCoreRuntime; + statusSink?: ZaloStatusSink; + fetcher?: ZaloFetch; +}; +type ZaloPollingLoopParams = ZaloProcessingContext & { + abortSignal: AbortSignal; + isStopped: () => boolean; + mediaMaxMb: number; +}; +type ZaloUpdateProcessingParams = ZaloProcessingContext & { + update: ZaloUpdate; + mediaMaxMb: number; +}; +type ZaloMessagePipelineParams = ZaloProcessingContext & { + message: ZaloMessage; + text?: string; + mediaPath?: string; + mediaType?: string; +}; +type ZaloImageMessageParams = ZaloProcessingContext & { + message: ZaloMessage; + mediaMaxMb: number; +}; function formatZaloError(error: unknown): string { if (error instanceof Error) { @@ -135,32 +164,21 @@ export async function handleZaloWebhookRequest( res: ServerResponse, ): Promise { return handleZaloWebhookRequestInternal(req, res, async ({ update, target }) => { - await processUpdate( + await processUpdate({ update, - target.token, - target.account, - target.config, - target.runtime, - target.core as ZaloCoreRuntime, - target.mediaMaxMb, - target.statusSink, - target.fetcher, - ); + token: target.token, + account: target.account, + config: target.config, + runtime: target.runtime, + core: target.core as ZaloCoreRuntime, + mediaMaxMb: target.mediaMaxMb, + statusSink: target.statusSink, + fetcher: target.fetcher, + }); }); } -function startPollingLoop(params: { - token: string; - account: ResolvedZaloAccount; - config: OpenClawConfig; - runtime: ZaloRuntimeEnv; - core: ZaloCoreRuntime; - abortSignal: AbortSignal; - isStopped: () => boolean; - mediaMaxMb: number; - statusSink?: (patch: { lastInboundAt?: number; lastOutboundAt?: number }) => void; - fetcher?: ZaloFetch; -}) { +function startPollingLoop(params: ZaloPollingLoopParams) { const { token, account, @@ -174,6 +192,16 @@ function startPollingLoop(params: { fetcher, } = params; const pollTimeout = 30; + const processingContext = { + token, + account, + config, + runtime, + core, + mediaMaxMb, + statusSink, + fetcher, + }; runtime.log?.(`[${account.accountId}] Zalo polling loop started timeout=${String(pollTimeout)}s`); @@ -186,17 +214,10 @@ function startPollingLoop(params: { const response = await getUpdates(token, { timeout: pollTimeout }, fetcher); if (response.ok && response.result) { statusSink?.({ lastInboundAt: Date.now() }); - await processUpdate( - response.result, - token, - account, - config, - runtime, - core, - mediaMaxMb, - statusSink, - fetcher, - ); + await processUpdate({ + update: response.result, + ...processingContext, + }); } } catch (err) { if (err instanceof ZaloApiError && err.isPollingTimeout) { @@ -215,38 +236,27 @@ function startPollingLoop(params: { void poll(); } -async function processUpdate( - update: ZaloUpdate, - token: string, - account: ResolvedZaloAccount, - config: OpenClawConfig, - runtime: ZaloRuntimeEnv, - core: ZaloCoreRuntime, - mediaMaxMb: number, - statusSink?: (patch: { lastInboundAt?: number; lastOutboundAt?: number }) => void, - fetcher?: ZaloFetch, -): Promise { +async function processUpdate(params: ZaloUpdateProcessingParams): Promise { + const { update, token, account, config, runtime, core, mediaMaxMb, statusSink, fetcher } = params; const { event_name, message } = update; + const sharedContext = { token, account, config, runtime, core, statusSink, fetcher }; if (!message) { return; } switch (event_name) { case "message.text.received": - await handleTextMessage(message, token, account, config, runtime, core, statusSink, fetcher); + await handleTextMessage({ + message, + ...sharedContext, + }); break; case "message.image.received": - await handleImageMessage( + await handleImageMessage({ message, - token, - account, - config, - runtime, - core, + ...sharedContext, mediaMaxMb, - statusSink, - fetcher, - ); + }); break; case "message.sticker.received": logVerbose(core, runtime, `[${account.accountId}] Received sticker from ${message.from.id}`); @@ -262,46 +272,24 @@ async function processUpdate( } async function handleTextMessage( - message: ZaloMessage, - token: string, - account: ResolvedZaloAccount, - config: OpenClawConfig, - runtime: ZaloRuntimeEnv, - core: ZaloCoreRuntime, - statusSink?: (patch: { lastInboundAt?: number; lastOutboundAt?: number }) => void, - fetcher?: ZaloFetch, + params: ZaloProcessingContext & { message: ZaloMessage }, ): Promise { + const { message } = params; const { text } = message; if (!text?.trim()) { return; } await processMessageWithPipeline({ - message, - token, - account, - config, - runtime, - core, + ...params, text, mediaPath: undefined, mediaType: undefined, - statusSink, - fetcher, }); } -async function handleImageMessage( - message: ZaloMessage, - token: string, - account: ResolvedZaloAccount, - config: OpenClawConfig, - runtime: ZaloRuntimeEnv, - core: ZaloCoreRuntime, - mediaMaxMb: number, - statusSink?: (patch: { lastInboundAt?: number; lastOutboundAt?: number }) => void, - fetcher?: ZaloFetch, -): Promise { +async function handleImageMessage(params: ZaloImageMessageParams): Promise { + const { message, mediaMaxMb, account, core, runtime } = params; const { photo, caption } = message; let mediaPath: string | undefined; @@ -325,33 +313,14 @@ async function handleImageMessage( } await processMessageWithPipeline({ - message, - token, - account, - config, - runtime, - core, + ...params, text: caption, mediaPath, mediaType, - statusSink, - fetcher, }); } -async function processMessageWithPipeline(params: { - message: ZaloMessage; - token: string; - account: ResolvedZaloAccount; - config: OpenClawConfig; - runtime: ZaloRuntimeEnv; - core: ZaloCoreRuntime; - text?: string; - mediaPath?: string; - mediaType?: string; - statusSink?: (patch: { lastInboundAt?: number; lastOutboundAt?: number }) => void; - fetcher?: ZaloFetch; -}): Promise { +async function processMessageWithPipeline(params: ZaloMessagePipelineParams): Promise { const { message, token, @@ -609,7 +578,7 @@ async function deliverZaloReply(params: { core: ZaloCoreRuntime; config: OpenClawConfig; accountId?: string; - statusSink?: (patch: { lastInboundAt?: number; lastOutboundAt?: number }) => void; + statusSink?: ZaloStatusSink; fetcher?: ZaloFetch; tableMode?: MarkdownTableMode; }): Promise { diff --git a/extensions/zalo/src/send.ts b/extensions/zalo/src/send.ts index 44f1549067a..4f35f242191 100644 --- a/extensions/zalo/src/send.ts +++ b/extensions/zalo/src/send.ts @@ -21,6 +21,28 @@ export type ZaloSendResult = { error?: string; }; +function toZaloSendResult(response: { + ok?: boolean; + result?: { message_id?: string }; +}): ZaloSendResult { + if (response.ok && response.result) { + return { ok: true, messageId: response.result.message_id }; + } + return { ok: false, error: "Failed to send message" }; +} + +async function runZaloSend( + failureMessage: string, + send: () => Promise<{ ok?: boolean; result?: { message_id?: string } }>, +): Promise { + try { + const result = toZaloSendResult(await send()); + return result.ok ? result : { ok: false, error: failureMessage }; + } catch (err) { + return { ok: false, error: err instanceof Error ? err.message : String(err) }; + } +} + function resolveSendContext(options: ZaloSendOptions): { token: string; fetcher?: ZaloFetch; @@ -73,24 +95,16 @@ export async function sendMessageZalo( }); } - try { - const response = await sendMessage( + return await runZaloSend("Failed to send message", () => + sendMessage( context.token, { chat_id: context.chatId, text: text.slice(0, 2000), }, context.fetcher, - ); - - if (response.ok && response.result) { - return { ok: true, messageId: response.result.message_id }; - } - - return { ok: false, error: "Failed to send message" }; - } catch (err) { - return { ok: false, error: err instanceof Error ? err.message : String(err) }; - } + ), + ); } export async function sendPhotoZalo( @@ -107,8 +121,8 @@ export async function sendPhotoZalo( return { ok: false, error: "No photo URL provided" }; } - try { - const response = await sendPhoto( + return await runZaloSend("Failed to send photo", () => + sendPhoto( context.token, { chat_id: context.chatId, @@ -116,14 +130,6 @@ export async function sendPhotoZalo( caption: options.caption?.slice(0, 2000), }, context.fetcher, - ); - - if (response.ok && response.result) { - return { ok: true, messageId: response.result.message_id }; - } - - return { ok: false, error: "Failed to send photo" }; - } catch (err) { - return { ok: false, error: err instanceof Error ? err.message : String(err) }; - } + ), + ); } diff --git a/src/browser/chrome-mcp.test.ts b/src/browser/chrome-mcp.test.ts index 3b64054c407..ec6f21339ed 100644 --- a/src/browser/chrome-mcp.test.ts +++ b/src/browser/chrome-mcp.test.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { + evaluateChromeMcpScript, listChromeMcpTabs, openChromeMcpTab, resetChromeMcpSessionsForTest, @@ -48,6 +49,16 @@ function createFakeSession(): ChromeMcpSession { ], }; } + if (name === "evaluate_script") { + return { + content: [ + { + type: "text", + text: "```json\n123\n```", + }, + ], + }; + } throw new Error(`unexpected tool ${name}`); }); @@ -105,4 +116,17 @@ describe("chrome MCP page parsing", () => { type: "page", }); }); + + it("parses evaluate_script text responses when structuredContent is missing", async () => { + const factory: ChromeMcpSessionFactory = async () => createFakeSession(); + setChromeMcpSessionFactoryForTest(factory); + + const result = await evaluateChromeMcpScript({ + profileName: "chrome-live", + targetId: "1", + fn: "() => 123", + }); + + expect(result).toBe(123); + }); }); diff --git a/src/browser/chrome-mcp.ts b/src/browser/chrome-mcp.ts index 7719a2338e3..ecd196547d3 100644 --- a/src/browser/chrome-mcp.ts +++ b/src/browser/chrome-mcp.ts @@ -33,6 +33,8 @@ const DEFAULT_CHROME_MCP_ARGS = [ "-y", "chrome-devtools-mcp@latest", "--autoConnect", + // Direct chrome-devtools-mcp launches do not enable structuredContent by default. + "--experimentalStructuredContent", "--experimental-page-id-routing", ]; @@ -133,6 +135,33 @@ function extractJsonBlock(text: string): unknown { return raw ? JSON.parse(raw) : null; } +function extractMessageText(result: ChromeMcpToolResult): string { + const message = extractStructuredContent(result).message; + if (typeof message === "string" && message.trim()) { + return message; + } + const blocks = extractTextContent(result); + return blocks.find((block) => block.trim()) ?? ""; +} + +function extractJsonMessage(result: ChromeMcpToolResult): unknown { + const candidates = [extractMessageText(result), ...extractTextContent(result)].filter((text) => + text.trim(), + ); + let lastError: unknown; + for (const candidate of candidates) { + try { + return extractJsonBlock(candidate); + } catch (err) { + lastError = err; + } + } + if (lastError) { + throw lastError; + } + return null; +} + async function createRealSession(profileName: string): Promise { const transport = new StdioClientTransport({ command: DEFAULT_CHROME_MCP_COMMAND, @@ -457,12 +486,7 @@ export async function evaluateChromeMcpScript(params: { function: params.fn, ...(params.args?.length ? { args: params.args } : {}), }); - const message = extractStructuredContent(result).message; - const text = typeof message === "string" ? message : ""; - if (!text.trim()) { - return null; - } - return extractJsonBlock(text); + return extractJsonMessage(result); } export async function waitForChromeMcpText(params: { diff --git a/src/browser/client-actions-core.ts b/src/browser/client-actions-core.ts index 72e27cd9afa..149ca54fadf 100644 --- a/src/browser/client-actions-core.ts +++ b/src/browser/client-actions-core.ts @@ -15,16 +15,19 @@ export type BrowserFormField = { export type BrowserActRequest = | { kind: "click"; - ref: string; + ref?: string; + selector?: string; targetId?: string; doubleClick?: boolean; button?: string; modifiers?: string[]; + delayMs?: number; timeoutMs?: number; } | { kind: "type"; - ref: string; + ref?: string; + selector?: string; text: string; targetId?: string; submit?: boolean; @@ -32,23 +35,33 @@ export type BrowserActRequest = timeoutMs?: number; } | { kind: "press"; key: string; targetId?: string; delayMs?: number } - | { kind: "hover"; ref: string; targetId?: string; timeoutMs?: number } + | { + kind: "hover"; + ref?: string; + selector?: string; + targetId?: string; + timeoutMs?: number; + } | { kind: "scrollIntoView"; - ref: string; + ref?: string; + selector?: string; targetId?: string; timeoutMs?: number; } | { kind: "drag"; - startRef: string; - endRef: string; + startRef?: string; + startSelector?: string; + endRef?: string; + endSelector?: string; targetId?: string; timeoutMs?: number; } | { kind: "select"; - ref: string; + ref?: string; + selector?: string; values: string[]; targetId?: string; timeoutMs?: number; @@ -73,13 +86,20 @@ export type BrowserActRequest = timeoutMs?: number; } | { kind: "evaluate"; fn: string; ref?: string; targetId?: string; timeoutMs?: number } - | { kind: "close"; targetId?: string }; + | { kind: "close"; targetId?: string } + | { + kind: "batch"; + actions: BrowserActRequest[]; + targetId?: string; + stopOnError?: boolean; + }; export type BrowserActResponse = { ok: true; targetId: string; url?: string; result?: unknown; + results?: Array<{ ok: boolean; error?: string }>; }; export type BrowserDownloadPayload = { diff --git a/src/browser/client.test.ts b/src/browser/client.test.ts index a4f95c23007..64d37580e35 100644 --- a/src/browser/client.test.ts +++ b/src/browser/client.test.ts @@ -160,6 +160,7 @@ describe("browser client", () => { targetId: "t1", url: "https://x", result: 1, + results: [{ ok: true }], }), } as unknown as Response; } @@ -258,7 +259,7 @@ describe("browser client", () => { ).resolves.toMatchObject({ ok: true, targetId: "t1" }); await expect( browserAct("http://127.0.0.1:18791", { kind: "click", ref: "1" }), - ).resolves.toMatchObject({ ok: true, targetId: "t1" }); + ).resolves.toMatchObject({ ok: true, targetId: "t1", results: [{ ok: true }] }); await expect( browserArmFileChooser("http://127.0.0.1:18791", { paths: ["/tmp/a.txt"], diff --git a/src/browser/profiles-service.ts b/src/browser/profiles-service.ts index 936a55c1ffa..25c0461f795 100644 --- a/src/browser/profiles-service.ts +++ b/src/browser/profiles-service.ts @@ -6,7 +6,6 @@ import { deriveDefaultBrowserCdpPortRange } from "../config/port-defaults.js"; import { isLoopbackHost } from "../gateway/net.js"; import { resolveOpenClawUserDataDir } from "./chrome.js"; import { parseHttpUrl, resolveProfile } from "./config.js"; -import { DEFAULT_BROWSER_DEFAULT_PROFILE_NAME } from "./constants.js"; import { BrowserConflictError, BrowserProfileNotFoundError, @@ -110,7 +109,12 @@ export function createBrowserProfilesService(ctx: BrowserRouteContext) { let profileConfig: BrowserProfileConfig; if (rawCdpUrl) { - const parsed = parseHttpUrl(rawCdpUrl, "browser.profiles.cdpUrl"); + let parsed: ReturnType; + try { + parsed = parseHttpUrl(rawCdpUrl, "browser.profiles.cdpUrl"); + } catch (err) { + throw new BrowserValidationError(String(err)); + } if (driver === "extension") { if (!isLoopbackHost(parsed.parsed.hostname)) { throw new BrowserValidationError( @@ -189,21 +193,20 @@ export function createBrowserProfilesService(ctx: BrowserRouteContext) { throw new BrowserValidationError("invalid profile name"); } + const state = ctx.state(); const cfg = loadConfig(); const profiles = cfg.browser?.profiles ?? {}; - if (!(name in profiles)) { - throw new BrowserProfileNotFoundError(`profile "${name}" not found`); - } - - const defaultProfile = cfg.browser?.defaultProfile ?? DEFAULT_BROWSER_DEFAULT_PROFILE_NAME; + const defaultProfile = cfg.browser?.defaultProfile ?? state.resolved.defaultProfile; if (name === defaultProfile) { throw new BrowserValidationError( `cannot delete the default profile "${name}"; change browser.defaultProfile first`, ); } + if (!(name in profiles)) { + throw new BrowserProfileNotFoundError(`profile "${name}" not found`); + } let deleted = false; - const state = ctx.state(); const resolved = resolveProfile(state.resolved, name); if (resolved?.cdpIsLoopback && resolved.driver === "openclaw") { diff --git a/src/browser/pw-ai.ts b/src/browser/pw-ai.ts index 6da8b410c83..f8d538b5394 100644 --- a/src/browser/pw-ai.ts +++ b/src/browser/pw-ai.ts @@ -19,6 +19,7 @@ export { export { armDialogViaPlaywright, armFileUploadViaPlaywright, + batchViaPlaywright, clickViaPlaywright, closePageViaPlaywright, cookiesClearViaPlaywright, diff --git a/src/browser/pw-tools-core.interactions.batch.test.ts b/src/browser/pw-tools-core.interactions.batch.test.ts new file mode 100644 index 00000000000..fbd2de4cbc6 --- /dev/null +++ b/src/browser/pw-tools-core.interactions.batch.test.ts @@ -0,0 +1,104 @@ +import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; + +let page: { evaluate: ReturnType } | null = null; + +const getPageForTargetId = vi.fn(async () => { + if (!page) { + throw new Error("test: page not set"); + } + return page; +}); +const ensurePageState = vi.fn(() => {}); +const forceDisconnectPlaywrightForTarget = vi.fn(async () => {}); +const refLocator = vi.fn(() => { + throw new Error("test: refLocator should not be called"); +}); +const restoreRoleRefsForTarget = vi.fn(() => {}); + +const closePageViaPlaywright = vi.fn(async () => {}); +const resizeViewportViaPlaywright = vi.fn(async () => {}); + +vi.mock("./pw-session.js", () => ({ + ensurePageState, + forceDisconnectPlaywrightForTarget, + getPageForTargetId, + refLocator, + restoreRoleRefsForTarget, +})); + +vi.mock("./pw-tools-core.snapshot.js", () => ({ + closePageViaPlaywright, + resizeViewportViaPlaywright, +})); + +let batchViaPlaywright: typeof import("./pw-tools-core.interactions.js").batchViaPlaywright; + +describe("batchViaPlaywright", () => { + beforeAll(async () => { + ({ batchViaPlaywright } = await import("./pw-tools-core.interactions.js")); + }); + + beforeEach(() => { + vi.clearAllMocks(); + page = { + evaluate: vi.fn(async () => "ok"), + }; + }); + + it("propagates evaluate timeouts through batched execution", async () => { + const result = await batchViaPlaywright({ + cdpUrl: "http://127.0.0.1:9222", + targetId: "tab-1", + evaluateEnabled: true, + actions: [{ kind: "evaluate", fn: "() => 1", timeoutMs: 5000 }], + }); + + expect(result).toEqual({ results: [{ ok: true }] }); + expect(page?.evaluate).toHaveBeenCalledWith( + expect.any(Function), + expect.objectContaining({ + fnBody: "() => 1", + timeoutMs: 4500, + }), + ); + }); + + it("supports resize and close inside a batch", async () => { + const result = await batchViaPlaywright({ + cdpUrl: "http://127.0.0.1:9222", + targetId: "tab-1", + actions: [{ kind: "resize", width: 800, height: 600 }, { kind: "close" }], + }); + + expect(result).toEqual({ results: [{ ok: true }, { ok: true }] }); + expect(resizeViewportViaPlaywright).toHaveBeenCalledWith({ + cdpUrl: "http://127.0.0.1:9222", + targetId: "tab-1", + width: 800, + height: 600, + }); + expect(closePageViaPlaywright).toHaveBeenCalledWith({ + cdpUrl: "http://127.0.0.1:9222", + targetId: "tab-1", + }); + }); + + it("propagates nested batch failures to the parent batch result", async () => { + const result = await batchViaPlaywright({ + cdpUrl: "http://127.0.0.1:9222", + targetId: "tab-1", + actions: [ + { + kind: "batch", + actions: [{ kind: "evaluate", fn: "() => 1" }], + }, + ], + }); + + expect(result).toEqual({ + results: [ + { ok: false, error: "act:evaluate is disabled by config (browser.evaluateEnabled=false)" }, + ], + }); + }); +}); diff --git a/src/browser/pw-tools-core.interactions.ts b/src/browser/pw-tools-core.interactions.ts index 852b11bb6dc..da0efa0c145 100644 --- a/src/browser/pw-tools-core.interactions.ts +++ b/src/browser/pw-tools-core.interactions.ts @@ -1,4 +1,4 @@ -import type { BrowserFormField } from "./client-actions-core.js"; +import type { BrowserActRequest, BrowserFormField } from "./client-actions-core.js"; import { DEFAULT_FILL_FIELD_TYPE } from "./form-fields.js"; import { DEFAULT_UPLOAD_DIR, resolveStrictExistingPathsWithinRoot } from "./paths.js"; import { @@ -8,12 +8,32 @@ import { refLocator, restoreRoleRefsForTarget, } from "./pw-session.js"; -import { normalizeTimeoutMs, requireRef, toAIFriendlyError } from "./pw-tools-core.shared.js"; +import { + normalizeTimeoutMs, + requireRef, + requireRefOrSelector, + toAIFriendlyError, +} from "./pw-tools-core.shared.js"; +import { closePageViaPlaywright, resizeViewportViaPlaywright } from "./pw-tools-core.snapshot.js"; type TargetOpts = { cdpUrl: string; targetId?: string; }; +const MAX_CLICK_DELAY_MS = 5_000; +const MAX_WAIT_TIME_MS = 30_000; +const MAX_BATCH_ACTIONS = 100; + +function resolveBoundedDelayMs(value: number | undefined, label: string, maxMs: number): number { + const normalized = Math.floor(value ?? 0); + if (!Number.isFinite(normalized) || normalized < 0) { + throw new Error(`${label} must be >= 0`); + } + if (normalized > maxMs) { + throw new Error(`${label} exceeds maximum of ${maxMs}ms`); + } + return normalized; +} async function getRestoredPageForTarget(opts: TargetOpts) { const page = await getPageForTargetId(opts); @@ -59,17 +79,27 @@ export async function highlightViaPlaywright(opts: { export async function clickViaPlaywright(opts: { cdpUrl: string; targetId?: string; - ref: string; + ref?: string; + selector?: string; doubleClick?: boolean; button?: "left" | "right" | "middle"; modifiers?: Array<"Alt" | "Control" | "ControlOrMeta" | "Meta" | "Shift">; + delayMs?: number; timeoutMs?: number; }): Promise { + const resolved = requireRefOrSelector(opts.ref, opts.selector); const page = await getRestoredPageForTarget(opts); - const ref = requireRef(opts.ref); - const locator = refLocator(page, ref); + const label = resolved.ref ?? resolved.selector!; + const locator = resolved.ref + ? refLocator(page, requireRef(resolved.ref)) + : page.locator(resolved.selector!); const timeout = resolveInteractionTimeoutMs(opts.timeoutMs); try { + const delayMs = resolveBoundedDelayMs(opts.delayMs, "click delayMs", MAX_CLICK_DELAY_MS); + if (delayMs > 0) { + await locator.hover({ timeout }); + await new Promise((r) => setTimeout(r, delayMs)); + } if (opts.doubleClick) { await locator.dblclick({ timeout, @@ -84,67 +114,84 @@ export async function clickViaPlaywright(opts: { }); } } catch (err) { - throw toAIFriendlyError(err, ref); + throw toAIFriendlyError(err, label); } } export async function hoverViaPlaywright(opts: { cdpUrl: string; targetId?: string; - ref: string; + ref?: string; + selector?: string; timeoutMs?: number; }): Promise { - const ref = requireRef(opts.ref); + const resolved = requireRefOrSelector(opts.ref, opts.selector); const page = await getRestoredPageForTarget(opts); + const label = resolved.ref ?? resolved.selector!; + const locator = resolved.ref + ? refLocator(page, requireRef(resolved.ref)) + : page.locator(resolved.selector!); try { - await refLocator(page, ref).hover({ + await locator.hover({ timeout: resolveInteractionTimeoutMs(opts.timeoutMs), }); } catch (err) { - throw toAIFriendlyError(err, ref); + throw toAIFriendlyError(err, label); } } export async function dragViaPlaywright(opts: { cdpUrl: string; targetId?: string; - startRef: string; - endRef: string; + startRef?: string; + startSelector?: string; + endRef?: string; + endSelector?: string; timeoutMs?: number; }): Promise { - const startRef = requireRef(opts.startRef); - const endRef = requireRef(opts.endRef); - if (!startRef || !endRef) { - throw new Error("startRef and endRef are required"); - } + const resolvedStart = requireRefOrSelector(opts.startRef, opts.startSelector); + const resolvedEnd = requireRefOrSelector(opts.endRef, opts.endSelector); const page = await getRestoredPageForTarget(opts); + const startLocator = resolvedStart.ref + ? refLocator(page, requireRef(resolvedStart.ref)) + : page.locator(resolvedStart.selector!); + const endLocator = resolvedEnd.ref + ? refLocator(page, requireRef(resolvedEnd.ref)) + : page.locator(resolvedEnd.selector!); + const startLabel = resolvedStart.ref ?? resolvedStart.selector!; + const endLabel = resolvedEnd.ref ?? resolvedEnd.selector!; try { - await refLocator(page, startRef).dragTo(refLocator(page, endRef), { + await startLocator.dragTo(endLocator, { timeout: resolveInteractionTimeoutMs(opts.timeoutMs), }); } catch (err) { - throw toAIFriendlyError(err, `${startRef} -> ${endRef}`); + throw toAIFriendlyError(err, `${startLabel} -> ${endLabel}`); } } export async function selectOptionViaPlaywright(opts: { cdpUrl: string; targetId?: string; - ref: string; + ref?: string; + selector?: string; values: string[]; timeoutMs?: number; }): Promise { - const ref = requireRef(opts.ref); + const resolved = requireRefOrSelector(opts.ref, opts.selector); if (!opts.values?.length) { throw new Error("values are required"); } const page = await getRestoredPageForTarget(opts); + const label = resolved.ref ?? resolved.selector!; + const locator = resolved.ref + ? refLocator(page, requireRef(resolved.ref)) + : page.locator(resolved.selector!); try { - await refLocator(page, ref).selectOption(opts.values, { + await locator.selectOption(opts.values, { timeout: resolveInteractionTimeoutMs(opts.timeoutMs), }); } catch (err) { - throw toAIFriendlyError(err, ref); + throw toAIFriendlyError(err, label); } } @@ -168,16 +215,20 @@ export async function pressKeyViaPlaywright(opts: { export async function typeViaPlaywright(opts: { cdpUrl: string; targetId?: string; - ref: string; + ref?: string; + selector?: string; text: string; submit?: boolean; slowly?: boolean; timeoutMs?: number; }): Promise { + const resolved = requireRefOrSelector(opts.ref, opts.selector); const text = String(opts.text ?? ""); const page = await getRestoredPageForTarget(opts); - const ref = requireRef(opts.ref); - const locator = refLocator(page, ref); + const label = resolved.ref ?? resolved.selector!; + const locator = resolved.ref + ? refLocator(page, requireRef(resolved.ref)) + : page.locator(resolved.selector!); const timeout = resolveInteractionTimeoutMs(opts.timeoutMs); try { if (opts.slowly) { @@ -190,7 +241,7 @@ export async function typeViaPlaywright(opts: { await locator.press("Enter", { timeout }); } } catch (err) { - throw toAIFriendlyError(err, ref); + throw toAIFriendlyError(err, label); } } @@ -367,18 +418,22 @@ export async function evaluateViaPlaywright(opts: { export async function scrollIntoViewViaPlaywright(opts: { cdpUrl: string; targetId?: string; - ref: string; + ref?: string; + selector?: string; timeoutMs?: number; }): Promise { + const resolved = requireRefOrSelector(opts.ref, opts.selector); const page = await getRestoredPageForTarget(opts); const timeout = normalizeTimeoutMs(opts.timeoutMs, 20_000); - const ref = requireRef(opts.ref); - const locator = refLocator(page, ref); + const label = resolved.ref ?? resolved.selector!; + const locator = resolved.ref + ? refLocator(page, requireRef(resolved.ref)) + : page.locator(resolved.selector!); try { await locator.scrollIntoViewIfNeeded({ timeout }); } catch (err) { - throw toAIFriendlyError(err, ref); + throw toAIFriendlyError(err, label); } } @@ -399,7 +454,7 @@ export async function waitForViaPlaywright(opts: { const timeout = normalizeTimeoutMs(opts.timeoutMs, 20_000); if (typeof opts.timeMs === "number" && Number.isFinite(opts.timeMs)) { - await page.waitForTimeout(Math.max(0, opts.timeMs)); + await page.waitForTimeout(resolveBoundedDelayMs(opts.timeMs, "wait timeMs", MAX_WAIT_TIME_MS)); } if (opts.text) { await page.getByText(opts.text).first().waitFor({ @@ -648,3 +703,193 @@ export async function setInputFilesViaPlaywright(opts: { // Best-effort for sites that don't react to setInputFiles alone. } } + +const MAX_BATCH_DEPTH = 5; + +async function executeSingleAction( + action: BrowserActRequest, + cdpUrl: string, + targetId?: string, + evaluateEnabled?: boolean, + depth = 0, +): Promise { + if (depth > MAX_BATCH_DEPTH) { + throw new Error(`Batch nesting depth exceeds maximum of ${MAX_BATCH_DEPTH}`); + } + const effectiveTargetId = action.targetId ?? targetId; + switch (action.kind) { + case "click": + await clickViaPlaywright({ + cdpUrl, + targetId: effectiveTargetId, + ref: action.ref, + selector: action.selector, + doubleClick: action.doubleClick, + button: action.button as "left" | "right" | "middle" | undefined, + modifiers: action.modifiers as Array< + "Alt" | "Control" | "ControlOrMeta" | "Meta" | "Shift" + >, + delayMs: action.delayMs, + timeoutMs: action.timeoutMs, + }); + break; + case "type": + await typeViaPlaywright({ + cdpUrl, + targetId: effectiveTargetId, + ref: action.ref, + selector: action.selector, + text: action.text, + submit: action.submit, + slowly: action.slowly, + timeoutMs: action.timeoutMs, + }); + break; + case "press": + await pressKeyViaPlaywright({ + cdpUrl, + targetId: effectiveTargetId, + key: action.key, + delayMs: action.delayMs, + }); + break; + case "hover": + await hoverViaPlaywright({ + cdpUrl, + targetId: effectiveTargetId, + ref: action.ref, + selector: action.selector, + timeoutMs: action.timeoutMs, + }); + break; + case "scrollIntoView": + await scrollIntoViewViaPlaywright({ + cdpUrl, + targetId: effectiveTargetId, + ref: action.ref, + selector: action.selector, + timeoutMs: action.timeoutMs, + }); + break; + case "drag": + await dragViaPlaywright({ + cdpUrl, + targetId: effectiveTargetId, + startRef: action.startRef, + startSelector: action.startSelector, + endRef: action.endRef, + endSelector: action.endSelector, + timeoutMs: action.timeoutMs, + }); + break; + case "select": + await selectOptionViaPlaywright({ + cdpUrl, + targetId: effectiveTargetId, + ref: action.ref, + selector: action.selector, + values: action.values, + timeoutMs: action.timeoutMs, + }); + break; + case "fill": + await fillFormViaPlaywright({ + cdpUrl, + targetId: effectiveTargetId, + fields: action.fields, + timeoutMs: action.timeoutMs, + }); + break; + case "resize": + await resizeViewportViaPlaywright({ + cdpUrl, + targetId: effectiveTargetId, + width: action.width, + height: action.height, + }); + break; + case "wait": + if (action.fn && !evaluateEnabled) { + throw new Error("wait --fn is disabled by config (browser.evaluateEnabled=false)"); + } + await waitForViaPlaywright({ + cdpUrl, + targetId: effectiveTargetId, + timeMs: action.timeMs, + text: action.text, + textGone: action.textGone, + selector: action.selector, + url: action.url, + loadState: action.loadState, + fn: action.fn, + timeoutMs: action.timeoutMs, + }); + break; + case "evaluate": + if (!evaluateEnabled) { + throw new Error("act:evaluate is disabled by config (browser.evaluateEnabled=false)"); + } + await evaluateViaPlaywright({ + cdpUrl, + targetId: effectiveTargetId, + fn: action.fn, + ref: action.ref, + timeoutMs: action.timeoutMs, + }); + break; + case "close": + await closePageViaPlaywright({ + cdpUrl, + targetId: effectiveTargetId, + }); + break; + case "batch": + // Nested batches: delegate recursively + const nestedResult = await batchViaPlaywright({ + cdpUrl, + targetId: effectiveTargetId, + actions: action.actions, + stopOnError: action.stopOnError, + evaluateEnabled, + depth: depth + 1, + }); + const nestedFailure = nestedResult.results.find((result) => !result.ok); + if (nestedFailure) { + throw new Error(nestedFailure.error ?? "Nested batch action failed"); + } + break; + default: + throw new Error(`Unsupported batch action kind: ${(action as { kind: string }).kind}`); + } +} + +export async function batchViaPlaywright(opts: { + cdpUrl: string; + targetId?: string; + actions: BrowserActRequest[]; + stopOnError?: boolean; + evaluateEnabled?: boolean; + depth?: number; +}): Promise<{ results: Array<{ ok: boolean; error?: string }> }> { + const depth = opts.depth ?? 0; + if (depth > MAX_BATCH_DEPTH) { + throw new Error(`Batch nesting depth exceeds maximum of ${MAX_BATCH_DEPTH}`); + } + if (opts.actions.length > MAX_BATCH_ACTIONS) { + throw new Error(`Batch exceeds maximum of ${MAX_BATCH_ACTIONS} actions`); + } + const results: Array<{ ok: boolean; error?: string }> = []; + for (const action of opts.actions) { + try { + await executeSingleAction(action, opts.cdpUrl, opts.targetId, opts.evaluateEnabled, depth); + results.push({ ok: true }); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + results.push({ ok: false, error: message }); + if (opts.stopOnError !== false) { + break; + } + } + } + return { results }; +} diff --git a/src/browser/pw-tools-core.shared.ts b/src/browser/pw-tools-core.shared.ts index d5ad74477d4..b6132de92bf 100644 --- a/src/browser/pw-tools-core.shared.ts +++ b/src/browser/pw-tools-core.shared.ts @@ -29,6 +29,21 @@ export function requireRef(value: unknown): string { return ref; } +export function requireRefOrSelector( + ref: string | undefined, + selector: string | undefined, +): { ref?: string; selector?: string } { + const trimmedRef = typeof ref === "string" ? ref.trim() : ""; + const trimmedSelector = typeof selector === "string" ? selector.trim() : ""; + if (!trimmedRef && !trimmedSelector) { + throw new Error("ref or selector is required"); + } + return { + ref: trimmedRef || undefined, + selector: trimmedSelector || undefined, + }; +} + export function normalizeTimeoutMs(timeoutMs: number | undefined, fallback: number) { return Math.max(500, Math.min(120_000, timeoutMs ?? fallback)); } diff --git a/src/browser/resolved-config-refresh.ts b/src/browser/resolved-config-refresh.ts index fe934069a80..999a7ca1229 100644 --- a/src/browser/resolved-config-refresh.ts +++ b/src/browser/resolved-config-refresh.ts @@ -1,4 +1,4 @@ -import { createConfigIO, loadConfig } from "../config/config.js"; +import { createConfigIO, getRuntimeConfigSnapshot } from "../config/config.js"; import { resolveBrowserConfig, resolveProfile, type ResolvedBrowserProfile } from "./config.js"; import type { BrowserServerState } from "./server-context.types.js"; @@ -29,7 +29,13 @@ function applyResolvedConfig( current: BrowserServerState, freshResolved: BrowserServerState["resolved"], ) { - current.resolved = freshResolved; + current.resolved = { + ...freshResolved, + // Keep the runtime evaluate gate stable across request-time profile refreshes. + // Security-sensitive behavior should only change via full runtime config reload, + // not as a side effect of resolving profiles/tabs during a request. + evaluateEnabled: current.resolved.evaluateEnabled, + }; for (const [name, runtime] of current.profiles) { const nextProfile = resolveProfile(freshResolved, name); if (nextProfile) { @@ -63,7 +69,11 @@ export function refreshResolvedBrowserConfigFromDisk(params: { if (!params.refreshConfigFromDisk) { return; } - const cfg = params.mode === "fresh" ? createConfigIO().loadConfig() : loadConfig(); + + // Route-level browser config hot reload should observe on-disk changes immediately. + // The shared loadConfig() helper may return a cached snapshot for the configured TTL, + // which can leave request-time browser guards stale (for example evaluateEnabled). + const cfg = getRuntimeConfigSnapshot() ?? createConfigIO().loadConfig(); const freshResolved = resolveBrowserConfig(cfg.browser, cfg); applyResolvedConfig(params.current, freshResolved); } diff --git a/src/browser/routes/agent.act.shared.ts b/src/browser/routes/agent.act.shared.ts index 81ca8caab71..b22f35e7ef2 100644 --- a/src/browser/routes/agent.act.shared.ts +++ b/src/browser/routes/agent.act.shared.ts @@ -1,4 +1,5 @@ export const ACT_KINDS = [ + "batch", "click", "close", "drag", diff --git a/src/browser/routes/agent.act.ts b/src/browser/routes/agent.act.ts index 8928a8a7d06..a8d3a09ce83 100644 --- a/src/browser/routes/agent.act.ts +++ b/src/browser/routes/agent.act.ts @@ -9,7 +9,7 @@ import { pressChromeMcpKey, resizeChromeMcpPage, } from "../chrome-mcp.js"; -import type { BrowserFormField } from "../client-actions-core.js"; +import type { BrowserActRequest, BrowserFormField } from "../client-actions-core.js"; import { normalizeBrowserFormField } from "../form-fields.js"; import type { BrowserRouteContext } from "../server-context.js"; import { registerBrowserAgentActDownloadRoutes } from "./agent.act.download.js"; @@ -34,6 +34,15 @@ function sleep(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); } +function browserEvaluateDisabledMessage(action: "wait" | "evaluate"): string { + return [ + action === "wait" + ? "wait --fn is disabled by config (browser.evaluateEnabled=false)." + : "act:evaluate is disabled by config (browser.evaluateEnabled=false).", + "Docs: /gateway/configuration#browser-openclaw-managed-browser", + ].join("\n"); +} + function buildExistingSessionWaitPredicate(params: { text?: string; textGone?: string; @@ -57,7 +66,7 @@ function buildExistingSessionWaitPredicate(params: { } if (params.loadState === "domcontentloaded") { checks.push(`document.readyState === "interactive" || document.readyState === "complete"`); - } else if (params.loadState === "load" || params.loadState === "networkidle") { + } else if (params.loadState === "load") { checks.push(`document.readyState === "complete"`); } if (params.fn) { @@ -104,6 +113,326 @@ async function waitForExistingSessionCondition(params: { throw new Error("Timed out waiting for condition"); } +const SELECTOR_ALLOWED_KINDS: ReadonlySet = new Set([ + "batch", + "click", + "drag", + "hover", + "scrollIntoView", + "select", + "type", + "wait", +]); +const MAX_BATCH_ACTIONS = 100; +const MAX_BATCH_CLICK_DELAY_MS = 5_000; +const MAX_BATCH_WAIT_TIME_MS = 30_000; + +function normalizeBoundedNonNegativeMs( + value: unknown, + fieldName: string, + maxMs: number, +): number | undefined { + const ms = toNumber(value); + if (ms === undefined) { + return undefined; + } + if (ms < 0) { + throw new Error(`${fieldName} must be >= 0`); + } + const normalized = Math.floor(ms); + if (normalized > maxMs) { + throw new Error(`${fieldName} exceeds maximum of ${maxMs}ms`); + } + return normalized; +} + +function countBatchActions(actions: BrowserActRequest[]): number { + let count = 0; + for (const action of actions) { + count += 1; + if (action.kind === "batch") { + count += countBatchActions(action.actions); + } + } + return count; +} + +function validateBatchTargetIds(actions: BrowserActRequest[], targetId: string): string | null { + for (const action of actions) { + if (action.targetId && action.targetId !== targetId) { + return "batched action targetId must match request targetId"; + } + if (action.kind === "batch") { + const nestedError = validateBatchTargetIds(action.actions, targetId); + if (nestedError) { + return nestedError; + } + } + } + return null; +} + +function normalizeBatchAction(value: unknown): BrowserActRequest { + if (!value || typeof value !== "object" || Array.isArray(value)) { + throw new Error("batch actions must be objects"); + } + const raw = value as Record; + const kind = toStringOrEmpty(raw.kind); + if (!isActKind(kind)) { + throw new Error("batch actions must use a supported kind"); + } + + switch (kind) { + case "click": { + const ref = toStringOrEmpty(raw.ref) || undefined; + const selector = toStringOrEmpty(raw.selector) || undefined; + if (!ref && !selector) { + throw new Error("click requires ref or selector"); + } + const buttonRaw = toStringOrEmpty(raw.button); + const button = buttonRaw ? parseClickButton(buttonRaw) : undefined; + if (buttonRaw && !button) { + throw new Error("click button must be left|right|middle"); + } + const modifiersRaw = toStringArray(raw.modifiers) ?? []; + const parsedModifiers = parseClickModifiers(modifiersRaw); + if (parsedModifiers.error) { + throw new Error(parsedModifiers.error); + } + const doubleClick = toBoolean(raw.doubleClick); + const delayMs = normalizeBoundedNonNegativeMs( + raw.delayMs, + "click delayMs", + MAX_BATCH_CLICK_DELAY_MS, + ); + const timeoutMs = toNumber(raw.timeoutMs); + const targetId = toStringOrEmpty(raw.targetId) || undefined; + return { + kind, + ...(ref ? { ref } : {}), + ...(selector ? { selector } : {}), + ...(targetId ? { targetId } : {}), + ...(doubleClick !== undefined ? { doubleClick } : {}), + ...(button ? { button } : {}), + ...(parsedModifiers.modifiers ? { modifiers: parsedModifiers.modifiers } : {}), + ...(delayMs !== undefined ? { delayMs } : {}), + ...(timeoutMs !== undefined ? { timeoutMs } : {}), + }; + } + case "type": { + const ref = toStringOrEmpty(raw.ref) || undefined; + const selector = toStringOrEmpty(raw.selector) || undefined; + const text = raw.text; + if (!ref && !selector) { + throw new Error("type requires ref or selector"); + } + if (typeof text !== "string") { + throw new Error("type requires text"); + } + const targetId = toStringOrEmpty(raw.targetId) || undefined; + const submit = toBoolean(raw.submit); + const slowly = toBoolean(raw.slowly); + const timeoutMs = toNumber(raw.timeoutMs); + return { + kind, + ...(ref ? { ref } : {}), + ...(selector ? { selector } : {}), + text, + ...(targetId ? { targetId } : {}), + ...(submit !== undefined ? { submit } : {}), + ...(slowly !== undefined ? { slowly } : {}), + ...(timeoutMs !== undefined ? { timeoutMs } : {}), + }; + } + case "press": { + const key = toStringOrEmpty(raw.key); + if (!key) { + throw new Error("press requires key"); + } + const targetId = toStringOrEmpty(raw.targetId) || undefined; + const delayMs = toNumber(raw.delayMs); + return { + kind, + key, + ...(targetId ? { targetId } : {}), + ...(delayMs !== undefined ? { delayMs } : {}), + }; + } + case "hover": + case "scrollIntoView": { + const ref = toStringOrEmpty(raw.ref) || undefined; + const selector = toStringOrEmpty(raw.selector) || undefined; + if (!ref && !selector) { + throw new Error(`${kind} requires ref or selector`); + } + const targetId = toStringOrEmpty(raw.targetId) || undefined; + const timeoutMs = toNumber(raw.timeoutMs); + return { + kind, + ...(ref ? { ref } : {}), + ...(selector ? { selector } : {}), + ...(targetId ? { targetId } : {}), + ...(timeoutMs !== undefined ? { timeoutMs } : {}), + }; + } + case "drag": { + const startRef = toStringOrEmpty(raw.startRef) || undefined; + const startSelector = toStringOrEmpty(raw.startSelector) || undefined; + const endRef = toStringOrEmpty(raw.endRef) || undefined; + const endSelector = toStringOrEmpty(raw.endSelector) || undefined; + if (!startRef && !startSelector) { + throw new Error("drag requires startRef or startSelector"); + } + if (!endRef && !endSelector) { + throw new Error("drag requires endRef or endSelector"); + } + const targetId = toStringOrEmpty(raw.targetId) || undefined; + const timeoutMs = toNumber(raw.timeoutMs); + return { + kind, + ...(startRef ? { startRef } : {}), + ...(startSelector ? { startSelector } : {}), + ...(endRef ? { endRef } : {}), + ...(endSelector ? { endSelector } : {}), + ...(targetId ? { targetId } : {}), + ...(timeoutMs !== undefined ? { timeoutMs } : {}), + }; + } + case "select": { + const ref = toStringOrEmpty(raw.ref) || undefined; + const selector = toStringOrEmpty(raw.selector) || undefined; + const values = toStringArray(raw.values); + if ((!ref && !selector) || !values?.length) { + throw new Error("select requires ref/selector and values"); + } + const targetId = toStringOrEmpty(raw.targetId) || undefined; + const timeoutMs = toNumber(raw.timeoutMs); + return { + kind, + ...(ref ? { ref } : {}), + ...(selector ? { selector } : {}), + values, + ...(targetId ? { targetId } : {}), + ...(timeoutMs !== undefined ? { timeoutMs } : {}), + }; + } + case "fill": { + const rawFields = Array.isArray(raw.fields) ? raw.fields : []; + const fields = rawFields + .map((field) => { + if (!field || typeof field !== "object") { + return null; + } + return normalizeBrowserFormField(field as Record); + }) + .filter((field): field is BrowserFormField => field !== null); + if (!fields.length) { + throw new Error("fill requires fields"); + } + const targetId = toStringOrEmpty(raw.targetId) || undefined; + const timeoutMs = toNumber(raw.timeoutMs); + return { + kind, + fields, + ...(targetId ? { targetId } : {}), + ...(timeoutMs !== undefined ? { timeoutMs } : {}), + }; + } + case "resize": { + const width = toNumber(raw.width); + const height = toNumber(raw.height); + if (width === undefined || height === undefined) { + throw new Error("resize requires width and height"); + } + const targetId = toStringOrEmpty(raw.targetId) || undefined; + return { + kind, + width, + height, + ...(targetId ? { targetId } : {}), + }; + } + case "wait": { + const loadStateRaw = toStringOrEmpty(raw.loadState); + const loadState = + loadStateRaw === "load" || + loadStateRaw === "domcontentloaded" || + loadStateRaw === "networkidle" + ? loadStateRaw + : undefined; + const timeMs = normalizeBoundedNonNegativeMs( + raw.timeMs, + "wait timeMs", + MAX_BATCH_WAIT_TIME_MS, + ); + const text = toStringOrEmpty(raw.text) || undefined; + const textGone = toStringOrEmpty(raw.textGone) || undefined; + const selector = toStringOrEmpty(raw.selector) || undefined; + const url = toStringOrEmpty(raw.url) || undefined; + const fn = toStringOrEmpty(raw.fn) || undefined; + if (timeMs === undefined && !text && !textGone && !selector && !url && !loadState && !fn) { + throw new Error( + "wait requires at least one of: timeMs, text, textGone, selector, url, loadState, fn", + ); + } + const targetId = toStringOrEmpty(raw.targetId) || undefined; + const timeoutMs = toNumber(raw.timeoutMs); + return { + kind, + ...(timeMs !== undefined ? { timeMs } : {}), + ...(text ? { text } : {}), + ...(textGone ? { textGone } : {}), + ...(selector ? { selector } : {}), + ...(url ? { url } : {}), + ...(loadState ? { loadState } : {}), + ...(fn ? { fn } : {}), + ...(targetId ? { targetId } : {}), + ...(timeoutMs !== undefined ? { timeoutMs } : {}), + }; + } + case "evaluate": { + const fn = toStringOrEmpty(raw.fn); + if (!fn) { + throw new Error("evaluate requires fn"); + } + const ref = toStringOrEmpty(raw.ref) || undefined; + const targetId = toStringOrEmpty(raw.targetId) || undefined; + const timeoutMs = toNumber(raw.timeoutMs); + return { + kind, + fn, + ...(ref ? { ref } : {}), + ...(targetId ? { targetId } : {}), + ...(timeoutMs !== undefined ? { timeoutMs } : {}), + }; + } + case "close": { + const targetId = toStringOrEmpty(raw.targetId) || undefined; + return { + kind, + ...(targetId ? { targetId } : {}), + }; + } + case "batch": { + const actions = Array.isArray(raw.actions) ? raw.actions.map(normalizeBatchAction) : []; + if (!actions.length) { + throw new Error("batch requires actions"); + } + if (countBatchActions(actions) > MAX_BATCH_ACTIONS) { + throw new Error(`batch exceeds maximum of ${MAX_BATCH_ACTIONS} actions`); + } + const targetId = toStringOrEmpty(raw.targetId) || undefined; + const stopOnError = toBoolean(raw.stopOnError); + return { + kind, + actions, + ...(targetId ? { targetId } : {}), + ...(stopOnError !== undefined ? { stopOnError } : {}), + }; + } + } +} + export function registerBrowserAgentActRoutes( app: BrowserRouteRegistrar, ctx: BrowserRouteContext, @@ -116,9 +445,20 @@ export function registerBrowserAgentActRoutes( } const kind: ActKind = kindRaw; const targetId = resolveTargetIdFromBody(body); - if (Object.hasOwn(body, "selector") && kind !== "wait") { + if (Object.hasOwn(body, "selector") && !SELECTOR_ALLOWED_KINDS.has(kind)) { return jsonError(res, 400, SELECTOR_UNSUPPORTED_MESSAGE); } + const earlyFn = kind === "wait" || kind === "evaluate" ? toStringOrEmpty(body.fn) : ""; + if ( + (kind === "evaluate" || (kind === "wait" && earlyFn)) && + !ctx.state().resolved.evaluateEnabled + ) { + return jsonError( + res, + 403, + browserEvaluateDisabledMessage(kind === "evaluate" ? "evaluate" : "wait"), + ); + } await withRouteTabContext({ req, @@ -132,12 +472,14 @@ export function registerBrowserAgentActRoutes( switch (kind) { case "click": { - const ref = toStringOrEmpty(body.ref); - if (!ref) { - return jsonError(res, 400, "ref is required"); + const ref = toStringOrEmpty(body.ref) || undefined; + const selector = toStringOrEmpty(body.selector) || undefined; + if (!ref && !selector) { + return jsonError(res, 400, "ref or selector is required"); } const doubleClick = toBoolean(body.doubleClick) ?? false; const timeoutMs = toNumber(body.timeoutMs); + const delayMs = toNumber(body.delayMs); const buttonRaw = toStringOrEmpty(body.button) || ""; const button = buttonRaw ? parseClickButton(buttonRaw) : undefined; if (buttonRaw && !button) { @@ -151,6 +493,13 @@ export function registerBrowserAgentActRoutes( } const modifiers = parsedModifiers.modifiers; if (isExistingSession) { + if (selector) { + return jsonError( + res, + 501, + "existing-session click does not support selector targeting yet; use ref.", + ); + } if ((button && button !== "left") || (modifiers && modifiers.length > 0)) { return jsonError( res, @@ -161,7 +510,7 @@ export function registerBrowserAgentActRoutes( await clickChromeMcpElement({ profileName, targetId: tab.targetId, - uid: ref, + uid: ref!, doubleClick, }); return res.json({ ok: true, targetId: tab.targetId, url: tab.url }); @@ -173,15 +522,23 @@ export function registerBrowserAgentActRoutes( const clickRequest: Parameters[0] = { cdpUrl, targetId: tab.targetId, - ref, doubleClick, }; + if (ref) { + clickRequest.ref = ref; + } + if (selector) { + clickRequest.selector = selector; + } if (button) { clickRequest.button = button; } if (modifiers) { clickRequest.modifiers = modifiers; } + if (delayMs) { + clickRequest.delayMs = delayMs; + } if (timeoutMs) { clickRequest.timeoutMs = timeoutMs; } @@ -189,9 +546,10 @@ export function registerBrowserAgentActRoutes( return res.json({ ok: true, targetId: tab.targetId, url: tab.url }); } case "type": { - const ref = toStringOrEmpty(body.ref); - if (!ref) { - return jsonError(res, 400, "ref is required"); + const ref = toStringOrEmpty(body.ref) || undefined; + const selector = toStringOrEmpty(body.selector) || undefined; + if (!ref && !selector) { + return jsonError(res, 400, "ref or selector is required"); } if (typeof body.text !== "string") { return jsonError(res, 400, "text is required"); @@ -201,6 +559,13 @@ export function registerBrowserAgentActRoutes( const slowly = toBoolean(body.slowly) ?? false; const timeoutMs = toNumber(body.timeoutMs); if (isExistingSession) { + if (selector) { + return jsonError( + res, + 501, + "existing-session type does not support selector targeting yet; use ref.", + ); + } if (slowly) { return jsonError( res, @@ -211,7 +576,7 @@ export function registerBrowserAgentActRoutes( await fillChromeMcpElement({ profileName, targetId: tab.targetId, - uid: ref, + uid: ref!, value: text, }); if (submit) { @@ -230,11 +595,16 @@ export function registerBrowserAgentActRoutes( const typeRequest: Parameters[0] = { cdpUrl, targetId: tab.targetId, - ref, text, submit, slowly, }; + if (ref) { + typeRequest.ref = ref; + } + if (selector) { + typeRequest.selector = selector; + } if (timeoutMs) { typeRequest.timeoutMs = timeoutMs; } @@ -267,12 +637,20 @@ export function registerBrowserAgentActRoutes( return res.json({ ok: true, targetId: tab.targetId }); } case "hover": { - const ref = toStringOrEmpty(body.ref); - if (!ref) { - return jsonError(res, 400, "ref is required"); + const ref = toStringOrEmpty(body.ref) || undefined; + const selector = toStringOrEmpty(body.selector) || undefined; + if (!ref && !selector) { + return jsonError(res, 400, "ref or selector is required"); } const timeoutMs = toNumber(body.timeoutMs); if (isExistingSession) { + if (selector) { + return jsonError( + res, + 501, + "existing-session hover does not support selector targeting yet; use ref.", + ); + } if (timeoutMs) { return jsonError( res, @@ -280,7 +658,7 @@ export function registerBrowserAgentActRoutes( "existing-session hover does not support timeoutMs overrides.", ); } - await hoverChromeMcpElement({ profileName, targetId: tab.targetId, uid: ref }); + await hoverChromeMcpElement({ profileName, targetId: tab.targetId, uid: ref! }); return res.json({ ok: true, targetId: tab.targetId }); } const pw = await requirePwAi(res, `act:${kind}`); @@ -291,17 +669,26 @@ export function registerBrowserAgentActRoutes( cdpUrl, targetId: tab.targetId, ref, + selector, timeoutMs: timeoutMs ?? undefined, }); return res.json({ ok: true, targetId: tab.targetId }); } case "scrollIntoView": { - const ref = toStringOrEmpty(body.ref); - if (!ref) { - return jsonError(res, 400, "ref is required"); + const ref = toStringOrEmpty(body.ref) || undefined; + const selector = toStringOrEmpty(body.selector) || undefined; + if (!ref && !selector) { + return jsonError(res, 400, "ref or selector is required"); } const timeoutMs = toNumber(body.timeoutMs); if (isExistingSession) { + if (selector) { + return jsonError( + res, + 501, + "existing-session scrollIntoView does not support selector targeting yet; use ref.", + ); + } if (timeoutMs) { return jsonError( res, @@ -313,7 +700,7 @@ export function registerBrowserAgentActRoutes( profileName, targetId: tab.targetId, fn: `(el) => { el.scrollIntoView({ block: "center", inline: "center" }); return true; }`, - args: [ref], + args: [ref!], }); return res.json({ ok: true, targetId: tab.targetId }); } @@ -324,8 +711,13 @@ export function registerBrowserAgentActRoutes( const scrollRequest: Parameters[0] = { cdpUrl, targetId: tab.targetId, - ref, }; + if (ref) { + scrollRequest.ref = ref; + } + if (selector) { + scrollRequest.selector = selector; + } if (timeoutMs) { scrollRequest.timeoutMs = timeoutMs; } @@ -333,13 +725,25 @@ export function registerBrowserAgentActRoutes( return res.json({ ok: true, targetId: tab.targetId }); } case "drag": { - const startRef = toStringOrEmpty(body.startRef); - const endRef = toStringOrEmpty(body.endRef); - if (!startRef || !endRef) { - return jsonError(res, 400, "startRef and endRef are required"); + const startRef = toStringOrEmpty(body.startRef) || undefined; + const startSelector = toStringOrEmpty(body.startSelector) || undefined; + const endRef = toStringOrEmpty(body.endRef) || undefined; + const endSelector = toStringOrEmpty(body.endSelector) || undefined; + if (!startRef && !startSelector) { + return jsonError(res, 400, "startRef or startSelector is required"); + } + if (!endRef && !endSelector) { + return jsonError(res, 400, "endRef or endSelector is required"); } const timeoutMs = toNumber(body.timeoutMs); if (isExistingSession) { + if (startSelector || endSelector) { + return jsonError( + res, + 501, + "existing-session drag does not support selector targeting yet; use startRef/endRef.", + ); + } if (timeoutMs) { return jsonError( res, @@ -350,8 +754,8 @@ export function registerBrowserAgentActRoutes( await dragChromeMcpElement({ profileName, targetId: tab.targetId, - fromUid: startRef, - toUid: endRef, + fromUid: startRef!, + toUid: endRef!, }); return res.json({ ok: true, targetId: tab.targetId }); } @@ -363,19 +767,29 @@ export function registerBrowserAgentActRoutes( cdpUrl, targetId: tab.targetId, startRef, + startSelector, endRef, + endSelector, timeoutMs: timeoutMs ?? undefined, }); return res.json({ ok: true, targetId: tab.targetId }); } case "select": { - const ref = toStringOrEmpty(body.ref); + const ref = toStringOrEmpty(body.ref) || undefined; + const selector = toStringOrEmpty(body.selector) || undefined; const values = toStringArray(body.values); - if (!ref || !values?.length) { - return jsonError(res, 400, "ref and values are required"); + if ((!ref && !selector) || !values?.length) { + return jsonError(res, 400, "ref/selector and values are required"); } const timeoutMs = toNumber(body.timeoutMs); if (isExistingSession) { + if (selector) { + return jsonError( + res, + 501, + "existing-session select does not support selector targeting yet; use ref.", + ); + } if (values.length !== 1) { return jsonError( res, @@ -393,7 +807,7 @@ export function registerBrowserAgentActRoutes( await fillChromeMcpElement({ profileName, targetId: tab.targetId, - uid: ref, + uid: ref!, value: values[0] ?? "", }); return res.json({ ok: true, targetId: tab.targetId }); @@ -406,6 +820,7 @@ export function registerBrowserAgentActRoutes( cdpUrl, targetId: tab.targetId, ref, + selector, values, timeoutMs: timeoutMs ?? undefined, }); @@ -498,14 +913,7 @@ export function registerBrowserAgentActRoutes( const fn = toStringOrEmpty(body.fn) || undefined; const timeoutMs = toNumber(body.timeoutMs) ?? undefined; if (fn && !evaluateEnabled) { - return jsonError( - res, - 403, - [ - "wait --fn is disabled by config (browser.evaluateEnabled=false).", - "Docs: /gateway/configuration#browser-openclaw-managed-browser", - ].join("\n"), - ); + return jsonError(res, 403, browserEvaluateDisabledMessage("wait")); } if ( timeMs === undefined && @@ -523,6 +931,13 @@ export function registerBrowserAgentActRoutes( ); } if (isExistingSession) { + if (loadState === "networkidle") { + return jsonError( + res, + 501, + "existing-session wait does not support loadState=networkidle yet.", + ); + } await waitForExistingSessionCondition({ profileName, targetId: tab.targetId, @@ -557,14 +972,7 @@ export function registerBrowserAgentActRoutes( } case "evaluate": { if (!evaluateEnabled) { - return jsonError( - res, - 403, - [ - "act:evaluate is disabled by config (browser.evaluateEnabled=false).", - "Docs: /gateway/configuration#browser-openclaw-managed-browser", - ].join("\n"), - ); + return jsonError(res, 403, browserEvaluateDisabledMessage("evaluate")); } const fn = toStringOrEmpty(body.fn); if (!fn) { @@ -627,6 +1035,44 @@ export function registerBrowserAgentActRoutes( await pw.closePageViaPlaywright({ cdpUrl, targetId: tab.targetId }); return res.json({ ok: true, targetId: tab.targetId }); } + case "batch": { + if (isExistingSession) { + return jsonError( + res, + 501, + "existing-session batch is not supported yet; send actions individually.", + ); + } + const pw = await requirePwAi(res, `act:${kind}`); + if (!pw) { + return; + } + let actions: BrowserActRequest[]; + try { + actions = Array.isArray(body.actions) ? body.actions.map(normalizeBatchAction) : []; + } catch (err) { + return jsonError(res, 400, err instanceof Error ? err.message : String(err)); + } + if (!actions.length) { + return jsonError(res, 400, "actions are required"); + } + if (countBatchActions(actions) > MAX_BATCH_ACTIONS) { + return jsonError(res, 400, `batch exceeds maximum of ${MAX_BATCH_ACTIONS} actions`); + } + const targetIdError = validateBatchTargetIds(actions, tab.targetId); + if (targetIdError) { + return jsonError(res, 403, targetIdError); + } + const stopOnError = toBoolean(body.stopOnError) ?? true; + const result = await pw.batchViaPlaywright({ + cdpUrl, + targetId: tab.targetId, + actions, + stopOnError, + evaluateEnabled, + }); + return res.json({ ok: true, targetId: tab.targetId, results: result.results }); + } default: { return jsonError(res, 400, "unsupported kind"); } diff --git a/src/browser/routes/agent.existing-session.test.ts b/src/browser/routes/agent.existing-session.test.ts new file mode 100644 index 00000000000..077889d16f9 --- /dev/null +++ b/src/browser/routes/agent.existing-session.test.ts @@ -0,0 +1,198 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { registerBrowserAgentActRoutes } from "./agent.act.js"; +import { registerBrowserAgentSnapshotRoutes } from "./agent.snapshot.js"; +import type { + BrowserRequest, + BrowserResponse, + BrowserRouteHandler, + BrowserRouteRegistrar, +} from "./types.js"; + +const routeState = vi.hoisted(() => ({ + profileCtx: { + profile: { + driver: "existing-session" as const, + name: "chrome-live", + }, + ensureTabAvailable: vi.fn(async () => ({ + targetId: "7", + url: "https://example.com", + })), + }, + tab: { + targetId: "7", + url: "https://example.com", + }, +})); + +const chromeMcpMocks = vi.hoisted(() => ({ + evaluateChromeMcpScript: vi.fn(async () => true), + navigateChromeMcpPage: vi.fn(async ({ url }: { url: string }) => ({ url })), + takeChromeMcpScreenshot: vi.fn(async () => Buffer.from("png")), + takeChromeMcpSnapshot: vi.fn(async () => ({ + id: "root", + role: "document", + name: "Example", + children: [{ id: "btn-1", role: "button", name: "Continue" }], + })), +})); + +vi.mock("../chrome-mcp.js", () => ({ + clickChromeMcpElement: vi.fn(async () => {}), + closeChromeMcpTab: vi.fn(async () => {}), + dragChromeMcpElement: vi.fn(async () => {}), + evaluateChromeMcpScript: chromeMcpMocks.evaluateChromeMcpScript, + fillChromeMcpElement: vi.fn(async () => {}), + fillChromeMcpForm: vi.fn(async () => {}), + hoverChromeMcpElement: vi.fn(async () => {}), + navigateChromeMcpPage: chromeMcpMocks.navigateChromeMcpPage, + pressChromeMcpKey: vi.fn(async () => {}), + resizeChromeMcpPage: vi.fn(async () => {}), + takeChromeMcpScreenshot: chromeMcpMocks.takeChromeMcpScreenshot, + takeChromeMcpSnapshot: chromeMcpMocks.takeChromeMcpSnapshot, +})); + +vi.mock("../cdp.js", () => ({ + captureScreenshot: vi.fn(), + snapshotAria: vi.fn(), +})); + +vi.mock("../navigation-guard.js", () => ({ + assertBrowserNavigationAllowed: vi.fn(async () => {}), + assertBrowserNavigationResultAllowed: vi.fn(async () => {}), + withBrowserNavigationPolicy: vi.fn(() => ({})), +})); + +vi.mock("../screenshot.js", () => ({ + DEFAULT_BROWSER_SCREENSHOT_MAX_BYTES: 128, + DEFAULT_BROWSER_SCREENSHOT_MAX_SIDE: 64, + normalizeBrowserScreenshot: vi.fn(async (buffer: Buffer) => ({ + buffer, + contentType: "image/png", + })), +})); + +vi.mock("../../media/store.js", () => ({ + ensureMediaDir: vi.fn(async () => {}), + saveMediaBuffer: vi.fn(async () => ({ path: "/tmp/fake.png" })), +})); + +vi.mock("./agent.shared.js", () => ({ + getPwAiModule: vi.fn(async () => null), + handleRouteError: vi.fn(), + readBody: vi.fn((req: BrowserRequest) => req.body ?? {}), + requirePwAi: vi.fn(async () => { + throw new Error("Playwright should not be used for existing-session tests"); + }), + resolveProfileContext: vi.fn(() => routeState.profileCtx), + resolveTargetIdFromBody: vi.fn((body: Record) => + typeof body.targetId === "string" ? body.targetId : undefined, + ), + withPlaywrightRouteContext: vi.fn(), + withRouteTabContext: vi.fn(async ({ run }: { run: (args: unknown) => Promise }) => { + await run({ + profileCtx: routeState.profileCtx, + cdpUrl: "http://127.0.0.1:18800", + tab: routeState.tab, + }); + }), +})); + +function createApp() { + const getHandlers = new Map(); + const postHandlers = new Map(); + const deleteHandlers = new Map(); + const app: BrowserRouteRegistrar = { + get: (path, handler) => void getHandlers.set(path, handler), + post: (path, handler) => void postHandlers.set(path, handler), + delete: (path, handler) => void deleteHandlers.set(path, handler), + }; + return { app, getHandlers, postHandlers, deleteHandlers }; +} + +function createResponse() { + let statusCode = 200; + let jsonBody: unknown; + const res: BrowserResponse = { + status(code) { + statusCode = code; + return res; + }, + json(body) { + jsonBody = body; + }, + }; + return { + res, + get statusCode() { + return statusCode; + }, + get body() { + return jsonBody; + }, + }; +} + +describe("existing-session browser routes", () => { + beforeEach(() => { + routeState.profileCtx.ensureTabAvailable.mockClear(); + chromeMcpMocks.evaluateChromeMcpScript.mockReset(); + chromeMcpMocks.navigateChromeMcpPage.mockClear(); + chromeMcpMocks.takeChromeMcpScreenshot.mockClear(); + chromeMcpMocks.takeChromeMcpSnapshot.mockClear(); + chromeMcpMocks.evaluateChromeMcpScript + .mockResolvedValueOnce({ labels: 1, skipped: 0 } as never) + .mockResolvedValueOnce(true); + }); + + it("allows labeled AI snapshots for existing-session profiles", async () => { + const { app, getHandlers } = createApp(); + registerBrowserAgentSnapshotRoutes(app, { + state: () => ({ resolved: { ssrfPolicy: undefined } }), + } as never); + const handler = getHandlers.get("/snapshot"); + expect(handler).toBeTypeOf("function"); + + const response = createResponse(); + await handler?.({ params: {}, query: { format: "ai", labels: "1" } }, response.res); + + expect(response.statusCode).toBe(200); + expect(response.body).toMatchObject({ + ok: true, + format: "ai", + labels: true, + labelsCount: 1, + labelsSkipped: 0, + }); + expect(chromeMcpMocks.takeChromeMcpSnapshot).toHaveBeenCalledWith({ + profileName: "chrome-live", + targetId: "7", + }); + expect(chromeMcpMocks.takeChromeMcpScreenshot).toHaveBeenCalled(); + }); + + it("fails closed for existing-session networkidle waits", async () => { + const { app, postHandlers } = createApp(); + registerBrowserAgentActRoutes(app, { + state: () => ({ resolved: { evaluateEnabled: true } }), + } as never); + const handler = postHandlers.get("/act"); + expect(handler).toBeTypeOf("function"); + + const response = createResponse(); + await handler?.( + { + params: {}, + query: {}, + body: { kind: "wait", loadState: "networkidle" }, + }, + response.res, + ); + + expect(response.statusCode).toBe(501); + expect(response.body).toMatchObject({ + error: expect.stringContaining("loadState=networkidle"), + }); + expect(chromeMcpMocks.evaluateChromeMcpScript).not.toHaveBeenCalled(); + }); +}); diff --git a/src/browser/routes/agent.snapshot.ts b/src/browser/routes/agent.snapshot.ts index 1b8626141b5..3d090de149c 100644 --- a/src/browser/routes/agent.snapshot.ts +++ b/src/browser/routes/agent.snapshot.ts @@ -153,7 +153,10 @@ export async function resolveTargetIdAfterNavigate(opts: { }): Promise { let currentTargetId = opts.oldTargetId; try { - const pickReplacement = (tabs: Array<{ targetId: string; url: string }>) => { + const pickReplacement = ( + tabs: Array<{ targetId: string; url: string }>, + options?: { allowSingleTabFallback?: boolean }, + ) => { if (tabs.some((tab) => tab.targetId === opts.oldTargetId)) { return opts.oldTargetId; } @@ -165,7 +168,7 @@ export async function resolveTargetIdAfterNavigate(opts: { if (uniqueReplacement.length === 1) { return uniqueReplacement[0]?.targetId ?? opts.oldTargetId; } - if (tabs.length === 1) { + if (options?.allowSingleTabFallback && tabs.length === 1) { return tabs[0]?.targetId ?? opts.oldTargetId; } return opts.oldTargetId; @@ -174,7 +177,9 @@ export async function resolveTargetIdAfterNavigate(opts: { currentTargetId = pickReplacement(await opts.listTabs()); if (currentTargetId === opts.oldTargetId) { await new Promise((r) => setTimeout(r, 800)); - currentTargetId = pickReplacement(await opts.listTabs()); + currentTargetId = pickReplacement(await opts.listTabs(), { + allowSingleTabFallback: true, + }); } } catch { // Best-effort: fall back to pre-navigation targetId @@ -380,9 +385,6 @@ export function registerBrowserAgentSnapshotRoutes( return jsonError(res, 400, "labels/mode=efficient require format=ai"); } if (profileCtx.profile.driver === "existing-session") { - if (plan.labels) { - return jsonError(res, 501, "labels are not supported for existing-session profiles yet."); - } if (plan.selectorValue || plan.frameSelectorValue) { return jsonError( res, diff --git a/src/browser/server-context.hot-reload-profiles.test.ts b/src/browser/server-context.hot-reload-profiles.test.ts index ec0c7e072aa..f9eb2452ce2 100644 --- a/src/browser/server-context.hot-reload-profiles.test.ts +++ b/src/browser/server-context.hot-reload-profiles.test.ts @@ -30,6 +30,7 @@ vi.mock("../config/config.js", () => ({ return buildConfig(); }, }), + getRuntimeConfigSnapshot: () => null, loadConfig: () => { // simulate stale loadConfig that doesn't see updates unless cache cleared if (!cachedConfig) { diff --git a/src/browser/server.agent-contract-form-layout-act-commands.test.ts b/src/browser/server.agent-contract-form-layout-act-commands.test.ts index 738bf8b7e2d..c8b76c4b886 100644 --- a/src/browser/server.agent-contract-form-layout-act-commands.test.ts +++ b/src/browser/server.agent-contract-form-layout-act-commands.test.ts @@ -51,12 +51,14 @@ describe("browser control server", () => { values: ["a", "b"], }); expect(select.ok).toBe(true); - expect(pwMocks.selectOptionViaPlaywright).toHaveBeenCalledWith({ - cdpUrl: state.cdpBaseUrl, - targetId: "abcd1234", - ref: "5", - values: ["a", "b"], - }); + expect(pwMocks.selectOptionViaPlaywright).toHaveBeenCalledWith( + expect.objectContaining({ + cdpUrl: expect.any(String), + targetId: "abcd1234", + ref: "5", + values: ["a", "b"], + }), + ); const fillCases: Array<{ input: Record; @@ -81,11 +83,13 @@ describe("browser control server", () => { fields: [input], }); expect(fill.ok).toBe(true); - expect(pwMocks.fillFormViaPlaywright).toHaveBeenCalledWith({ - cdpUrl: state.cdpBaseUrl, - targetId: "abcd1234", - fields: [expected], - }); + expect(pwMocks.fillFormViaPlaywright).toHaveBeenCalledWith( + expect.objectContaining({ + cdpUrl: expect.any(String), + targetId: "abcd1234", + fields: [expected], + }), + ); } const resize = await postJson<{ ok: boolean }>(`${base}/act`, { @@ -94,12 +98,14 @@ describe("browser control server", () => { height: 600, }); expect(resize.ok).toBe(true); - expect(pwMocks.resizeViewportViaPlaywright).toHaveBeenCalledWith({ - cdpUrl: state.cdpBaseUrl, - targetId: "abcd1234", - width: 800, - height: 600, - }); + expect(pwMocks.resizeViewportViaPlaywright).toHaveBeenCalledWith( + expect.objectContaining({ + cdpUrl: expect.any(String), + targetId: "abcd1234", + width: 800, + height: 600, + }), + ); const wait = await postJson<{ ok: boolean }>(`${base}/act`, { kind: "wait", @@ -150,13 +156,152 @@ describe("browser control server", () => { kind: "evaluate", fn: "() => 1", }); - expect(res.error).toContain("browser.evaluateEnabled=false"); expect(pwMocks.evaluateViaPlaywright).not.toHaveBeenCalled(); }, slowTimeoutMs, ); + it( + "normalizes batch actions and threads evaluateEnabled into the batch executor", + async () => { + const base = await startServerAndBase(); + + const batchRes = await postJson<{ ok: boolean; results?: Array<{ ok: boolean }> }>( + `${base}/act`, + { + kind: "batch", + stopOnError: "false", + actions: [ + { kind: "click", selector: "button.save", doubleClick: "true", delayMs: "25" }, + { kind: "wait", fn: " () => window.ready === true " }, + ], + }, + ); + + expect(batchRes.ok).toBe(true); + expect(pwMocks.batchViaPlaywright).toHaveBeenCalledWith( + expect.objectContaining({ + cdpUrl: expect.any(String), + targetId: "abcd1234", + stopOnError: false, + evaluateEnabled: true, + actions: [ + { + kind: "click", + selector: "button.save", + doubleClick: true, + delayMs: 25, + }, + { + kind: "wait", + fn: "() => window.ready === true", + }, + ], + }), + ); + }, + slowTimeoutMs, + ); + + it( + "preserves exact type text in batch normalization", + async () => { + const base = await startServerAndBase(); + + const batchRes = await postJson<{ ok: boolean }>(`${base}/act`, { + kind: "batch", + actions: [ + { kind: "type", selector: "input.name", text: " padded " }, + { kind: "type", selector: "input.clearable", text: "" }, + ], + }); + + expect(batchRes.ok).toBe(true); + expect(pwMocks.batchViaPlaywright).toHaveBeenCalledWith( + expect.objectContaining({ + actions: [ + { + kind: "type", + selector: "input.name", + text: " padded ", + }, + { + kind: "type", + selector: "input.clearable", + text: "", + }, + ], + }), + ); + }, + slowTimeoutMs, + ); + + it( + "rejects malformed batch actions before dispatch", + async () => { + const base = await startServerAndBase(); + + const batchRes = await postJson<{ error?: string }>(`${base}/act`, { + kind: "batch", + actions: [{ kind: "click", ref: {} }], + }); + + expect(batchRes.error).toContain("click requires ref or selector"); + expect(pwMocks.batchViaPlaywright).not.toHaveBeenCalled(); + }, + slowTimeoutMs, + ); + + it( + "rejects batched action targetId overrides before dispatch", + async () => { + const base = await startServerAndBase(); + + const batchRes = await postJson<{ error?: string }>(`${base}/act`, { + kind: "batch", + actions: [{ kind: "click", ref: "5", targetId: "other-tab" }], + }); + + expect(batchRes.error).toContain("batched action targetId must match request targetId"); + expect(pwMocks.batchViaPlaywright).not.toHaveBeenCalled(); + }, + slowTimeoutMs, + ); + + it( + "rejects oversized batch delays before dispatch", + async () => { + const base = await startServerAndBase(); + + const batchRes = await postJson<{ error?: string }>(`${base}/act`, { + kind: "batch", + actions: [{ kind: "click", selector: "button.save", delayMs: 5001 }], + }); + + expect(batchRes.error).toContain("click delayMs exceeds maximum of 5000ms"); + expect(pwMocks.batchViaPlaywright).not.toHaveBeenCalled(); + }, + slowTimeoutMs, + ); + + it( + "rejects oversized top-level batches before dispatch", + async () => { + const base = await startServerAndBase(); + + const batchRes = await postJson<{ error?: string }>(`${base}/act`, { + kind: "batch", + actions: Array.from({ length: 101 }, () => ({ kind: "press", key: "Enter" })), + }); + + expect(batchRes.error).toContain("batch exceeds maximum of 100 actions"); + expect(pwMocks.batchViaPlaywright).not.toHaveBeenCalled(); + }, + slowTimeoutMs, + ); + it("agent contract: hooks + response + downloads + screenshot", async () => { const base = await startServerAndBase(); @@ -165,13 +310,15 @@ describe("browser control server", () => { timeoutMs: 1234, }); expect(upload).toMatchObject({ ok: true }); - expect(pwMocks.armFileUploadViaPlaywright).toHaveBeenCalledWith({ - cdpUrl: state.cdpBaseUrl, - targetId: "abcd1234", - // The server resolves paths (which adds a drive letter on Windows for `\\tmp\\...` style roots). - paths: [path.resolve(DEFAULT_UPLOAD_DIR, "a.txt")], - timeoutMs: 1234, - }); + expect(pwMocks.armFileUploadViaPlaywright).toHaveBeenCalledWith( + expect.objectContaining({ + cdpUrl: expect.any(String), + targetId: "abcd1234", + // The server resolves paths (which adds a drive letter on Windows for `\\tmp\\...` style roots). + paths: [path.resolve(DEFAULT_UPLOAD_DIR, "a.txt")], + timeoutMs: 1234, + }), + ); const uploadWithRef = await postJson(`${base}/hooks/file-chooser`, { paths: ["b.txt"], @@ -280,7 +427,7 @@ describe("browser control server", () => { expect(res.path).toContain("safe-trace.zip"); expect(pwMocks.traceStopViaPlaywright).toHaveBeenCalledWith( expect.objectContaining({ - cdpUrl: state.cdpBaseUrl, + cdpUrl: expect.any(String), targetId: "abcd1234", path: expect.stringContaining("safe-trace.zip"), }), @@ -369,7 +516,7 @@ describe("browser control server", () => { expect(res.ok).toBe(true); expect(pwMocks.waitForDownloadViaPlaywright).toHaveBeenCalledWith( expect.objectContaining({ - cdpUrl: state.cdpBaseUrl, + cdpUrl: expect.any(String), targetId: "abcd1234", path: expect.stringContaining("safe-wait.pdf"), }), @@ -385,7 +532,7 @@ describe("browser control server", () => { expect(res.ok).toBe(true); expect(pwMocks.downloadViaPlaywright).toHaveBeenCalledWith( expect.objectContaining({ - cdpUrl: state.cdpBaseUrl, + cdpUrl: expect.any(String), targetId: "abcd1234", ref: "e12", path: expect.stringContaining("safe-download.pdf"), diff --git a/src/browser/server.control-server.test-harness.ts b/src/browser/server.control-server.test-harness.ts index 5721d9eb17b..118c83dbb73 100644 --- a/src/browser/server.control-server.test-harness.ts +++ b/src/browser/server.control-server.test-harness.ts @@ -11,6 +11,17 @@ type HarnessState = { reachable: boolean; cfgAttachOnly: boolean; cfgEvaluateEnabled: boolean; + cfgDefaultProfile: string; + cfgProfiles: Record< + string, + { + cdpPort?: number; + cdpUrl?: string; + color: string; + driver?: "openclaw" | "extension" | "existing-session"; + attachOnly?: boolean; + } + >; createTargetId: string | null; prevGatewayPort: string | undefined; prevGatewayToken: string | undefined; @@ -23,6 +34,8 @@ const state: HarnessState = { reachable: false, cfgAttachOnly: false, cfgEvaluateEnabled: true, + cfgDefaultProfile: "openclaw", + cfgProfiles: {}, createTargetId: null, prevGatewayPort: undefined, prevGatewayToken: undefined, @@ -61,6 +74,14 @@ export function setBrowserControlServerReachable(reachable: boolean): void { state.reachable = reachable; } +export function setBrowserControlServerProfiles( + profiles: HarnessState["cfgProfiles"], + defaultProfile = Object.keys(profiles)[0] ?? "openclaw", +): void { + state.cfgProfiles = profiles; + state.cfgDefaultProfile = defaultProfile; +} + const cdpMocks = vi.hoisted(() => ({ createTargetViaCdp: vi.fn<() => Promise<{ targetId: string }>>(async () => { throw new Error("cdp disabled"); @@ -77,6 +98,7 @@ export function getCdpMocks(): { createTargetViaCdp: MockFn; snapshotAria: MockF const pwMocks = vi.hoisted(() => ({ armDialogViaPlaywright: vi.fn(async () => {}), armFileUploadViaPlaywright: vi.fn(async () => {}), + batchViaPlaywright: vi.fn(async () => ({ results: [] })), clickViaPlaywright: vi.fn(async () => {}), closePageViaPlaywright: vi.fn(async () => {}), closePlaywrightBrowserConnection: vi.fn(async () => {}), @@ -121,6 +143,44 @@ export function getPwMocks(): Record { return pwMocks as unknown as Record; } +const chromeMcpMocks = vi.hoisted(() => ({ + clickChromeMcpElement: vi.fn(async () => {}), + closeChromeMcpSession: vi.fn(async () => true), + closeChromeMcpTab: vi.fn(async () => {}), + dragChromeMcpElement: vi.fn(async () => {}), + ensureChromeMcpAvailable: vi.fn(async () => {}), + evaluateChromeMcpScript: vi.fn(async () => true), + fillChromeMcpElement: vi.fn(async () => {}), + fillChromeMcpForm: vi.fn(async () => {}), + focusChromeMcpTab: vi.fn(async () => {}), + getChromeMcpPid: vi.fn(() => 4321), + hoverChromeMcpElement: vi.fn(async () => {}), + listChromeMcpTabs: vi.fn(async () => [ + { targetId: "7", title: "", url: "https://example.com", type: "page" }, + ]), + navigateChromeMcpPage: vi.fn(async ({ url }: { url: string }) => ({ url })), + openChromeMcpTab: vi.fn(async (_profile: string, url: string) => ({ + targetId: "8", + title: "", + url, + type: "page", + })), + pressChromeMcpKey: vi.fn(async () => {}), + resizeChromeMcpPage: vi.fn(async () => {}), + takeChromeMcpScreenshot: vi.fn(async () => Buffer.from("png")), + takeChromeMcpSnapshot: vi.fn(async () => ({ + id: "root", + role: "document", + name: "Example", + children: [{ id: "btn-1", role: "button", name: "Continue" }], + })), + uploadChromeMcpFile: vi.fn(async () => {}), +})); + +export function getChromeMcpMocks(): Record { + return chromeMcpMocks as unknown as Record; +} + const chromeUserDataDir = vi.hoisted(() => ({ dir: "/tmp/openclaw" })); installChromeUserDataDirHooks(chromeUserDataDir); @@ -147,24 +207,40 @@ function makeProc(pid = 123) { const proc = makeProc(); +function defaultProfilesForState(testPort: number): HarnessState["cfgProfiles"] { + return { + openclaw: { cdpPort: testPort + 9, color: "#FF4500" }, + }; +} + vi.mock("../config/config.js", async (importOriginal) => { const actual = await importOriginal(); - return { - ...actual, - loadConfig: () => ({ + const loadConfig = () => { + return { browser: { enabled: true, evaluateEnabled: state.cfgEvaluateEnabled, color: "#FF4500", attachOnly: state.cfgAttachOnly, headless: true, - defaultProfile: "openclaw", - profiles: { - openclaw: { cdpPort: state.testPort + 1, color: "#FF4500" }, - }, + defaultProfile: state.cfgDefaultProfile, + profiles: + Object.keys(state.cfgProfiles).length > 0 + ? state.cfgProfiles + : defaultProfilesForState(state.testPort), }, - }), - writeConfigFile: vi.fn(async () => {}), + }; + }; + const writeConfigFile = vi.fn(async () => {}); + return { + ...actual, + createConfigIO: vi.fn(() => ({ + loadConfig, + writeConfigFile, + })), + getRuntimeConfigSnapshot: vi.fn(() => null), + loadConfig, + writeConfigFile, }; }); @@ -209,8 +285,12 @@ vi.mock("./cdp.js", () => ({ vi.mock("./pw-ai.js", () => pwMocks); +vi.mock("./chrome-mcp.js", () => chromeMcpMocks); + vi.mock("../media/store.js", () => ({ + MEDIA_MAX_BYTES: 5 * 1024 * 1024, ensureMediaDir: vi.fn(async () => {}), + getMediaDir: vi.fn(() => "/tmp"), saveMediaBuffer: vi.fn(async () => ({ path: "/tmp/fake.png" })), })); @@ -251,13 +331,18 @@ function mockClearAll(obj: Record unknown }>) { export async function resetBrowserControlServerTestContext(): Promise { state.reachable = false; state.cfgAttachOnly = false; + state.cfgEvaluateEnabled = true; + state.cfgDefaultProfile = "openclaw"; + state.cfgProfiles = defaultProfilesForState(state.testPort); state.createTargetId = null; mockClearAll(pwMocks); mockClearAll(cdpMocks); + mockClearAll(chromeMcpMocks); state.testPort = await getFreePort(); - state.cdpBaseUrl = `http://127.0.0.1:${state.testPort + 1}`; + state.cdpBaseUrl = `http://127.0.0.1:${state.testPort + 9}`; + state.cfgProfiles = defaultProfilesForState(state.testPort); state.prevGatewayPort = process.env.OPENCLAW_GATEWAY_PORT; process.env.OPENCLAW_GATEWAY_PORT = String(state.testPort - 2); // Avoid flaky auth coupling: some suites temporarily set gateway env auth diff --git a/src/cli/browser-cli-manage.timeout-option.test.ts b/src/cli/browser-cli-manage.timeout-option.test.ts index 134f13bc3c3..7338d97701e 100644 --- a/src/cli/browser-cli-manage.timeout-option.test.ts +++ b/src/cli/browser-cli-manage.timeout-option.test.ts @@ -76,4 +76,48 @@ describe("browser manage start timeout option", () => { expect(startCall?.[0]).toMatchObject({ timeout: "60000" }); expect(startCall?.[2]).toBeUndefined(); }); + + it("uses a longer built-in timeout for browser status", async () => { + const program = createProgram(); + await program.parseAsync(["browser", "status"], { from: "user" }); + + const statusCall = mocks.callBrowserRequest.mock.calls.find( + (call) => ((call[1] ?? {}) as { path?: string }).path === "/", + ) as [Record, { path?: string }, { timeoutMs?: number }] | undefined; + + expect(statusCall?.[2]).toEqual({ timeoutMs: 45_000 }); + }); + + it("uses a longer built-in timeout for browser tabs", async () => { + const program = createProgram(); + await program.parseAsync(["browser", "tabs"], { from: "user" }); + + const tabsCall = mocks.callBrowserRequest.mock.calls.find( + (call) => ((call[1] ?? {}) as { path?: string }).path === "/tabs", + ) as [Record, { path?: string }, { timeoutMs?: number }] | undefined; + + expect(tabsCall?.[2]).toEqual({ timeoutMs: 45_000 }); + }); + + it("uses a longer built-in timeout for browser profiles", async () => { + const program = createProgram(); + await program.parseAsync(["browser", "profiles"], { from: "user" }); + + const profilesCall = mocks.callBrowserRequest.mock.calls.find( + (call) => ((call[1] ?? {}) as { path?: string }).path === "/profiles", + ) as [Record, { path?: string }, { timeoutMs?: number }] | undefined; + + expect(profilesCall?.[2]).toEqual({ timeoutMs: 45_000 }); + }); + + it("uses a longer built-in timeout for browser open", async () => { + const program = createProgram(); + await program.parseAsync(["browser", "open", "https://example.com"], { from: "user" }); + + const openCall = mocks.callBrowserRequest.mock.calls.find( + (call) => ((call[1] ?? {}) as { path?: string }).path === "/tabs/open", + ) as [Record, { path?: string }, { timeoutMs?: number }] | undefined; + + expect(openCall?.[2]).toEqual({ timeoutMs: 45_000 }); + }); }); diff --git a/src/cli/browser-cli-manage.ts b/src/cli/browser-cli-manage.ts index 31d4b02c2aa..8fad97eaf38 100644 --- a/src/cli/browser-cli-manage.ts +++ b/src/cli/browser-cli-manage.ts @@ -13,6 +13,8 @@ import { shortenHomePath } from "../utils.js"; import { callBrowserRequest, type BrowserParentOpts } from "./browser-cli-shared.js"; import { runCommandWithRuntime } from "./cli-utils.js"; +const BROWSER_MANAGE_REQUEST_TIMEOUT_MS = 45_000; + function resolveProfileQuery(profile?: string) { return profile ? { profile } : undefined; } @@ -38,7 +40,7 @@ async function callTabAction( query: resolveProfileQuery(profile), body, }, - { timeoutMs: 10_000 }, + { timeoutMs: BROWSER_MANAGE_REQUEST_TIMEOUT_MS }, ); } @@ -54,7 +56,7 @@ async function fetchBrowserStatus( query: resolveProfileQuery(profile), }, { - timeoutMs: 1500, + timeoutMs: BROWSER_MANAGE_REQUEST_TIMEOUT_MS, }, ); } @@ -196,7 +198,7 @@ export function registerBrowserManageCommands( path: "/tabs", query: resolveProfileQuery(profile), }, - { timeoutMs: 3000 }, + { timeoutMs: BROWSER_MANAGE_REQUEST_TIMEOUT_MS }, ); const tabs = result.tabs ?? []; logBrowserTabs(tabs, parent?.json); @@ -220,7 +222,7 @@ export function registerBrowserManageCommands( action: "list", }, }, - { timeoutMs: 10_000 }, + { timeoutMs: BROWSER_MANAGE_REQUEST_TIMEOUT_MS }, ); const tabs = result.tabs ?? []; logBrowserTabs(tabs, parent?.json); @@ -305,7 +307,7 @@ export function registerBrowserManageCommands( query: resolveProfileQuery(profile), body: { url }, }, - { timeoutMs: 15000 }, + { timeoutMs: BROWSER_MANAGE_REQUEST_TIMEOUT_MS }, ); if (printJsonResult(parent, tab)) { return; @@ -330,7 +332,7 @@ export function registerBrowserManageCommands( query: resolveProfileQuery(profile), body: { targetId }, }, - { timeoutMs: 5000 }, + { timeoutMs: BROWSER_MANAGE_REQUEST_TIMEOUT_MS }, ); if (printJsonResult(parent, { ok: true })) { return; @@ -355,7 +357,7 @@ export function registerBrowserManageCommands( path: `/tabs/${encodeURIComponent(targetId.trim())}`, query: resolveProfileQuery(profile), }, - { timeoutMs: 5000 }, + { timeoutMs: BROWSER_MANAGE_REQUEST_TIMEOUT_MS }, ); } else { await callBrowserRequest( @@ -366,7 +368,7 @@ export function registerBrowserManageCommands( query: resolveProfileQuery(profile), body: { kind: "close" }, }, - { timeoutMs: 20000 }, + { timeoutMs: BROWSER_MANAGE_REQUEST_TIMEOUT_MS }, ); } if (printJsonResult(parent, { ok: true })) { @@ -389,7 +391,7 @@ export function registerBrowserManageCommands( method: "GET", path: "/profiles", }, - { timeoutMs: 3000 }, + { timeoutMs: BROWSER_MANAGE_REQUEST_TIMEOUT_MS }, ); const profiles = result.profiles ?? []; if (printJsonResult(parent, { profiles })) { diff --git a/src/cli/update-cli.test.ts b/src/cli/update-cli.test.ts index 34ca4efaa87..f2138215327 100644 --- a/src/cli/update-cli.test.ts +++ b/src/cli/update-cli.test.ts @@ -668,6 +668,54 @@ describe("update-cli", () => { expect(runDaemonInstall).not.toHaveBeenCalled(); }); + it("updateCommand reuses the captured invocation cwd when process.cwd later fails", async () => { + const root = createCaseDir("openclaw-updated-root"); + const entryPath = path.join(root, "dist", "entry.js"); + pathExists.mockImplementation(async (candidate: string) => candidate === entryPath); + + const originalCwd = process.cwd(); + let restoreCwd: (() => void) | undefined; + vi.mocked(runGatewayUpdate).mockImplementation(async () => { + const cwdSpy = vi.spyOn(process, "cwd").mockImplementation(() => { + throw new Error("ENOENT: current working directory is gone"); + }); + restoreCwd = () => cwdSpy.mockRestore(); + return { + status: "ok", + mode: "npm", + root, + steps: [], + durationMs: 100, + }; + }); + serviceLoaded.mockResolvedValue(true); + + try { + await withEnvAsync( + { + OPENCLAW_STATE_DIR: "./state", + }, + async () => { + await updateCommand({}); + }, + ); + } finally { + restoreCwd?.(); + } + + expect(runCommandWithTimeout).toHaveBeenCalledWith( + [expect.stringMatching(/node/), entryPath, "gateway", "install", "--force"], + expect.objectContaining({ + cwd: root, + env: expect.objectContaining({ + OPENCLAW_STATE_DIR: path.resolve(originalCwd, "./state"), + }), + timeoutMs: 60_000, + }), + ); + expect(runDaemonInstall).not.toHaveBeenCalled(); + }); + it("updateCommand falls back to restart when env refresh install fails", async () => { await runRestartFallbackScenario({ daemonInstall: "fail" }); }); diff --git a/src/cli/update-cli/update-command.ts b/src/cli/update-cli/update-command.ts index d0d39e0215a..b94fbd4ffb9 100644 --- a/src/cli/update-cli/update-command.ts +++ b/src/cli/update-cli/update-command.ts @@ -124,9 +124,17 @@ function formatCommandFailure(stdout: string, stderr: string): string { return detail.split("\n").slice(-3).join("\n"); } +function tryResolveInvocationCwd(): string | undefined { + try { + return process.cwd(); + } catch { + return undefined; + } +} + function resolveServiceRefreshEnv( env: NodeJS.ProcessEnv, - invocationCwd: string = process.cwd(), + invocationCwd?: string, ): NodeJS.ProcessEnv { const resolvedEnv: NodeJS.ProcessEnv = { ...env }; for (const key of SERVICE_REFRESH_PATH_ENV_KEYS) { @@ -138,6 +146,10 @@ function resolveServiceRefreshEnv( resolvedEnv[key] = rawValue; continue; } + if (!invocationCwd) { + resolvedEnv[key] = rawValue; + continue; + } resolvedEnv[key] = path.resolve(invocationCwd, rawValue); } return resolvedEnv; @@ -205,6 +217,7 @@ function printDryRunPreview(preview: UpdateDryRunPreview, jsonMode: boolean): vo async function refreshGatewayServiceEnv(params: { result: UpdateRunResult; jsonMode: boolean; + invocationCwd?: string; }): Promise { const args = ["gateway", "install", "--force"]; if (params.jsonMode) { @@ -217,7 +230,7 @@ async function refreshGatewayServiceEnv(params: { } const res = await runCommandWithTimeout([resolveNodeRunner(), candidate, ...args], { cwd: params.result.root, - env: resolveServiceRefreshEnv(process.env), + env: resolveServiceRefreshEnv(process.env, params.invocationCwd), timeoutMs: SERVICE_REFRESH_TIMEOUT_MS, }); if (res.code === 0) { @@ -547,6 +560,7 @@ async function maybeRestartService(params: { refreshServiceEnv: boolean; gatewayPort: number; restartScriptPath?: string | null; + invocationCwd?: string; }): Promise { if (params.shouldRestart) { if (!params.opts.json) { @@ -562,6 +576,7 @@ async function maybeRestartService(params: { await refreshGatewayServiceEnv({ result: params.result, jsonMode: Boolean(params.opts.json), + invocationCwd: params.invocationCwd, }); } catch (err) { if (!params.opts.json) { @@ -667,6 +682,7 @@ async function maybeRestartService(params: { export async function updateCommand(opts: UpdateCommandOptions): Promise { suppressDeprecations(); + const invocationCwd = tryResolveInvocationCwd(); const timeoutMs = parseTimeoutMsOrExit(opts.timeout); const shouldRestart = opts.restart !== false; @@ -949,6 +965,7 @@ export async function updateCommand(opts: UpdateCommandOptions): Promise { refreshServiceEnv: refreshGatewayServiceEnv, gatewayPort, restartScriptPath, + invocationCwd, }); if (!opts.json) { diff --git a/src/commands/onboard-non-interactive.gateway.test.ts b/src/commands/onboard-non-interactive.gateway.test.ts index c711b6922af..5396b20b9d6 100644 --- a/src/commands/onboard-non-interactive.gateway.test.ts +++ b/src/commands/onboard-non-interactive.gateway.test.ts @@ -1,6 +1,7 @@ import fs from "node:fs/promises"; import path from "node:path"; import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; +import type { RuntimeEnv } from "../runtime.js"; import { makeTempWorkspace } from "../test-helpers/workspace.js"; import { captureEnv } from "../test-utils/env.js"; import { createThrowingRuntime, readJsonFile } from "./onboard-non-interactive.test-helpers.js"; @@ -408,10 +409,16 @@ describe("onboard (non-interactive): gateway and remote auth", () => { })); let capturedError = ""; - const runtimeWithCapture = { - log: (..._args: unknown[]) => {}, + const runtimeWithCapture: RuntimeEnv = { + log: () => {}, error: (...args: unknown[]) => { - capturedError = args.map(String).join(" "); + const firstArg = args[0]; + capturedError = + typeof firstArg === "string" + ? firstArg + : firstArg instanceof Error + ? firstArg.message + : (JSON.stringify(firstArg) ?? ""); throw new Error(capturedError); }, exit: (_code: number) => { diff --git a/src/infra/executable-path.test.ts b/src/infra/executable-path.test.ts index 731457ab183..31437cafe49 100644 --- a/src/infra/executable-path.test.ts +++ b/src/infra/executable-path.test.ts @@ -47,4 +47,27 @@ describe("executable path helpers", () => { expect(resolveExecutablePath("runner", { env: { PATH: binDir } })).toBe(pathTool); expect(resolveExecutablePath("missing", { env: { PATH: binDir } })).toBeUndefined(); }); + + it("resolves absolute, home-relative, and Path-cased env executables", async () => { + const base = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-exec-path-")); + const homeDir = path.join(base, "home"); + const binDir = path.join(base, "bin"); + await fs.mkdir(homeDir, { recursive: true }); + await fs.mkdir(binDir, { recursive: true }); + + const homeTool = path.join(homeDir, "home-tool"); + const absoluteTool = path.join(base, "absolute-tool"); + const pathTool = path.join(binDir, "runner"); + await fs.writeFile(homeTool, "#!/bin/sh\nexit 0\n", "utf8"); + await fs.writeFile(absoluteTool, "#!/bin/sh\nexit 0\n", "utf8"); + await fs.writeFile(pathTool, "#!/bin/sh\nexit 0\n", "utf8"); + await fs.chmod(homeTool, 0o755); + await fs.chmod(absoluteTool, 0o755); + await fs.chmod(pathTool, 0o755); + + expect(resolveExecutablePath(absoluteTool)).toBe(absoluteTool); + expect(resolveExecutablePath("~/home-tool", { env: { HOME: homeDir } })).toBe(homeTool); + expect(resolveExecutablePath("runner", { env: { Path: binDir } })).toBe(pathTool); + expect(resolveExecutablePath("~/missing-tool", { env: { HOME: homeDir } })).toBeUndefined(); + }); }); diff --git a/src/infra/executable-path.ts b/src/infra/executable-path.ts index b25231a4a50..bf648c7cb6a 100644 --- a/src/infra/executable-path.ts +++ b/src/infra/executable-path.ts @@ -60,7 +60,9 @@ export function resolveExecutablePath( rawExecutable: string, options?: { cwd?: string; env?: NodeJS.ProcessEnv }, ): string | undefined { - const expanded = rawExecutable.startsWith("~") ? expandHomePrefix(rawExecutable) : rawExecutable; + const expanded = rawExecutable.startsWith("~") + ? expandHomePrefix(rawExecutable, { env: options?.env }) + : rawExecutable; if (expanded.includes("/") || expanded.includes("\\")) { if (path.isAbsolute(expanded)) { return isExecutableFile(expanded) ? expanded : undefined; diff --git a/src/infra/path-prepend.test.ts b/src/infra/path-prepend.test.ts index 29dfb504cfb..7f4211a0137 100644 --- a/src/infra/path-prepend.test.ts +++ b/src/infra/path-prepend.test.ts @@ -1,8 +1,20 @@ import path from "node:path"; import { describe, expect, it } from "vitest"; -import { mergePathPrepend, normalizePathPrepend } from "./path-prepend.js"; +import { + applyPathPrepend, + findPathKey, + mergePathPrepend, + normalizePathPrepend, +} from "./path-prepend.js"; describe("path prepend helpers", () => { + it("finds the actual PATH key while preserving original casing", () => { + expect(findPathKey({ PATH: "/usr/bin" })).toBe("PATH"); + expect(findPathKey({ Path: "/usr/bin" })).toBe("Path"); + expect(findPathKey({ PaTh: "/usr/bin" })).toBe("PaTh"); + expect(findPathKey({ HOME: "/tmp" })).toBe("PATH"); + }); + it("normalizes prepend lists by trimming, skipping blanks, and deduping", () => { expect( normalizePathPrepend([ @@ -30,4 +42,38 @@ describe("path prepend helpers", () => { mergePathPrepend(` /usr/bin ${path.delimiter} ${path.delimiter} /opt/bin `, ["/custom/bin"]), ).toBe(["/custom/bin", "/usr/bin", "/opt/bin"].join(path.delimiter)); }); + + it("applies prepends to the discovered PATH key and preserves existing casing", () => { + const env = { + Path: [`/usr/bin`, `/opt/bin`].join(path.delimiter), + }; + + applyPathPrepend(env, ["/custom/bin", "/usr/bin"]); + + expect(env).toEqual({ + Path: ["/custom/bin", "/usr/bin", "/opt/bin"].join(path.delimiter), + }); + }); + + it("respects requireExisting and ignores empty prepend lists", () => { + const envWithoutPath = { HOME: "/tmp/home" }; + applyPathPrepend(envWithoutPath, ["/custom/bin"], { requireExisting: true }); + expect(envWithoutPath).toEqual({ HOME: "/tmp/home" }); + + const envWithPath = { PATH: "/usr/bin" }; + applyPathPrepend(envWithPath, [], { requireExisting: true }); + applyPathPrepend(envWithPath, undefined, { requireExisting: true }); + expect(envWithPath).toEqual({ PATH: "/usr/bin" }); + }); + + it("creates PATH when prepends are provided and no path key exists", () => { + const env = { HOME: "/tmp/home" }; + + applyPathPrepend(env, ["/custom/bin"]); + + expect(env).toEqual({ + HOME: "/tmp/home", + PATH: "/custom/bin", + }); + }); }); diff --git a/src/infra/secret-file.test.ts b/src/infra/secret-file.test.ts index ca7841891e5..5e9e6fe7b90 100644 --- a/src/infra/secret-file.test.ts +++ b/src/infra/secret-file.test.ts @@ -29,6 +29,37 @@ describe("readSecretFileSync", () => { await writeFile(file, " top-secret \n", "utf8"); expect(readSecretFileSync(file, "Gateway password")).toBe("top-secret"); + expect(tryReadSecretFileSync(file, "Gateway password")).toBe("top-secret"); + }); + + it("surfaces resolvedPath and error details for missing files", async () => { + const dir = await createTempDir(); + const file = path.join(dir, "missing-secret.txt"); + + const result = loadSecretFileSync(file, "Gateway password"); + + expect(result).toMatchObject({ + ok: false, + resolvedPath: file, + message: expect.stringContaining(`Failed to inspect Gateway password file at ${file}:`), + error: expect.any(Error), + }); + }); + + it("preserves the underlying cause when throwing for missing files", async () => { + const dir = await createTempDir(); + const file = path.join(dir, "missing-secret.txt"); + + let thrown: Error | undefined; + try { + readSecretFileSync(file, "Gateway password"); + } catch (error) { + thrown = error as Error; + } + + expect(thrown).toBeInstanceOf(Error); + expect(thrown?.message).toContain(`Failed to inspect Gateway password file at ${file}:`); + expect((thrown as Error & { cause?: unknown }).cause).toBeInstanceOf(Error); }); it("rejects files larger than the secret-file limit", async () => { diff --git a/src/infra/shell-env.test.ts b/src/infra/shell-env.test.ts index 64be7f28fc3..52d65c9edc8 100644 --- a/src/infra/shell-env.test.ts +++ b/src/infra/shell-env.test.ts @@ -2,10 +2,12 @@ import fs from "node:fs"; import os from "node:os"; import { describe, expect, it, vi } from "vitest"; import { + getShellEnvAppliedKeys, getShellPathFromLoginShell, loadShellEnvFallback, resetShellPathCacheForTests, resolveShellEnvFallbackTimeoutMs, + shouldDeferShellEnvFallback, shouldEnableShellEnvFallback, } from "./shell-env.js"; @@ -119,6 +121,12 @@ describe("shell env fallback", () => { expect(shouldEnableShellEnvFallback({ OPENCLAW_LOAD_SHELL_ENV: "1" })).toBe(true); }); + it("uses the same truthy env parsing for deferred fallback", () => { + expect(shouldDeferShellEnvFallback({} as NodeJS.ProcessEnv)).toBe(false); + expect(shouldDeferShellEnvFallback({ OPENCLAW_DEFER_SHELL_ENV_FALLBACK: "false" })).toBe(false); + expect(shouldDeferShellEnvFallback({ OPENCLAW_DEFER_SHELL_ENV_FALLBACK: "yes" })).toBe(true); + }); + it("resolves timeout from env with default fallback", () => { expect(resolveShellEnvFallbackTimeoutMs({} as NodeJS.ProcessEnv)).toBe(15000); expect(resolveShellEnvFallbackTimeoutMs({ OPENCLAW_SHELL_ENV_TIMEOUT_MS: "42" })).toBe(42); @@ -179,6 +187,57 @@ describe("shell env fallback", () => { expect(exec2).not.toHaveBeenCalled(); }); + it("tracks last applied keys across success, skip, and failure paths", () => { + const successEnv: NodeJS.ProcessEnv = {}; + const successExec = vi.fn(() => + Buffer.from("OPENAI_API_KEY=from-shell\0DISCORD_BOT_TOKEN=\0EXTRA=ignored\0"), + ); + expect( + loadShellEnvFallback({ + enabled: true, + env: successEnv, + expectedKeys: ["OPENAI_API_KEY", "DISCORD_BOT_TOKEN"], + exec: successExec as unknown as Parameters[0]["exec"], + }), + ).toEqual({ + ok: true, + applied: ["OPENAI_API_KEY"], + }); + expect(getShellEnvAppliedKeys()).toEqual(["OPENAI_API_KEY"]); + + expect( + loadShellEnvFallback({ + enabled: false, + env: {}, + expectedKeys: ["OPENAI_API_KEY"], + exec: successExec as unknown as Parameters[0]["exec"], + }), + ).toEqual({ + ok: true, + applied: [], + skippedReason: "disabled", + }); + expect(getShellEnvAppliedKeys()).toEqual([]); + + const failureExec = vi.fn(() => { + throw new Error("boom"); + }); + expect( + loadShellEnvFallback({ + enabled: true, + env: {}, + expectedKeys: ["OPENAI_API_KEY"], + exec: failureExec as unknown as Parameters[0]["exec"], + logger: { warn: vi.fn() }, + }), + ).toMatchObject({ + ok: false, + applied: [], + error: "boom", + }); + expect(getShellEnvAppliedKeys()).toEqual([]); + }); + it("resolves PATH via login shell and caches it", () => { const exec = vi.fn(() => Buffer.from("PATH=/usr/local/bin:/usr/bin\0HOME=/tmp\0")); @@ -207,6 +266,19 @@ describe("shell env fallback", () => { expect(exec).toHaveBeenCalledOnce(); }); + it("returns null when login shell PATH is blank", () => { + const exec = vi.fn(() => Buffer.from("PATH= \0HOME=/tmp\0")); + + const { first, second } = probeShellPathWithFreshCache({ + exec, + platform: "linux", + }); + + expect(first).toBeNull(); + expect(second).toBeNull(); + expect(exec).toHaveBeenCalledOnce(); + }); + it("falls back to /bin/sh when SHELL is non-absolute", () => { const { res, exec } = runShellEnvFallbackForShell("zsh"); diff --git a/src/infra/tmp-openclaw-dir.test.ts b/src/infra/tmp-openclaw-dir.test.ts index 89056513856..72b67c95858 100644 --- a/src/infra/tmp-openclaw-dir.test.ts +++ b/src/infra/tmp-openclaw-dir.test.ts @@ -197,6 +197,27 @@ describe("resolvePreferredOpenClawTmpDir", () => { expectFallsBackToOsTmpDir({ lstatSync: vi.fn(() => makeDirStat({ mode: 0o40777 })) }); }); + it("repairs existing /tmp/openclaw permissions when they are too broad", () => { + let preferredMode = 0o40777; + const chmodSync = vi.fn((target: string, mode: number) => { + if (target === POSIX_OPENCLAW_TMP_DIR && mode === 0o700) { + preferredMode = 0o40700; + } + }); + const warn = vi.fn(); + + const { resolved, tmpdir } = resolveWithMocks({ + lstatSync: vi.fn(() => makeDirStat({ mode: preferredMode })), + chmodSync, + warn, + }); + + expect(resolved).toBe(POSIX_OPENCLAW_TMP_DIR); + expect(chmodSync).toHaveBeenCalledWith(POSIX_OPENCLAW_TMP_DIR, 0o700); + expect(warn).toHaveBeenCalledWith(expect.stringContaining("tightened permissions on temp dir")); + expect(tmpdir).not.toHaveBeenCalled(); + }); + it("throws when fallback path is a symlink", () => { const lstatSync = symlinkTmpDirLstat(); const fallbackLstatSync = vi.fn(() => makeDirStat({ isSymbolicLink: true, mode: 0o120777 })); @@ -222,6 +243,35 @@ describe("resolvePreferredOpenClawTmpDir", () => { expect(mkdirSync).toHaveBeenCalledWith(fallbackTmp(), { recursive: true, mode: 0o700 }); }); + it("uses an unscoped fallback suffix when process uid is unavailable", () => { + const tmpdirPath = "/var/fallback"; + const fallbackPath = path.join(tmpdirPath, "openclaw"); + + const resolved = resolvePreferredOpenClawTmpDir({ + accessSync: vi.fn((target: string) => { + if (target === "/tmp") { + throw new Error("read-only"); + } + }), + lstatSync: vi.fn((target: string) => { + if (target === POSIX_OPENCLAW_TMP_DIR) { + throw nodeErrorWithCode("ENOENT"); + } + if (target === fallbackPath) { + return makeDirStat({ uid: 0, mode: 0o40777 }); + } + return secureDirStat(); + }), + mkdirSync: vi.fn(), + chmodSync: vi.fn(), + getuid: vi.fn(() => undefined), + tmpdir: vi.fn(() => tmpdirPath), + warn: vi.fn(), + }); + + expect(resolved).toBe(fallbackPath); + }); + it("repairs fallback directory permissions after create when umask makes it group-writable", () => { const fallbackPath = fallbackTmp(); let fallbackMode = 0o40775; @@ -287,4 +337,25 @@ describe("resolvePreferredOpenClawTmpDir", () => { expect(chmodSync).toHaveBeenCalledWith(fallbackPath, 0o700); expect(warn).toHaveBeenCalledWith(expect.stringContaining("tightened permissions on temp dir")); }); + + it("throws when the fallback directory cannot be created", () => { + expect(() => + resolvePreferredOpenClawTmpDir({ + accessSync: readOnlyTmpAccessSync(), + lstatSync: vi.fn((target: string) => { + if (target === POSIX_OPENCLAW_TMP_DIR || target === fallbackTmp()) { + throw nodeErrorWithCode("ENOENT"); + } + return secureDirStat(); + }), + mkdirSync: vi.fn(() => { + throw new Error("mkdir failed"); + }), + chmodSync: vi.fn(), + getuid: vi.fn(() => 501), + tmpdir: vi.fn(() => "/var/fallback"), + warn: vi.fn(), + }), + ).toThrow(/Unable to create fallback OpenClaw temp dir/); + }); }); diff --git a/src/plugin-sdk/index.test.ts b/src/plugin-sdk/index.test.ts index 24cb7bb67e4..61d1cccb10c 100644 --- a/src/plugin-sdk/index.test.ts +++ b/src/plugin-sdk/index.test.ts @@ -1,6 +1,73 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { pathToFileURL } from "node:url"; +import { build } from "tsdown"; import { describe, expect, it } from "vitest"; import * as sdk from "./index.js"; +const pluginSdkEntrypoints = [ + "index", + "core", + "compat", + "telegram", + "discord", + "slack", + "signal", + "imessage", + "whatsapp", + "line", + "msteams", + "acpx", + "bluebubbles", + "copilot-proxy", + "device-pair", + "diagnostics-otel", + "diffs", + "feishu", + "google-gemini-cli-auth", + "googlechat", + "irc", + "llm-task", + "lobster", + "matrix", + "mattermost", + "memory-core", + "memory-lancedb", + "minimax-portal-auth", + "nextcloud-talk", + "nostr", + "open-prose", + "phone-control", + "qwen-portal-auth", + "synology-chat", + "talk-voice", + "test-utils", + "thread-ownership", + "tlon", + "twitch", + "voice-call", + "zalo", + "zalouser", + "account-id", + "keyed-async-queue", +] as const; + +const pluginSdkSpecifiers = pluginSdkEntrypoints.map((entry) => + entry === "index" ? "openclaw/plugin-sdk" : `openclaw/plugin-sdk/${entry}`, +); + +function buildPluginSdkPackageExports() { + return Object.fromEntries( + pluginSdkEntrypoints.map((entry) => [ + entry === "index" ? "./plugin-sdk" : `./plugin-sdk/${entry}`, + { + default: `./dist/plugin-sdk/${entry}.js`, + }, + ]), + ); +} + describe("plugin-sdk exports", () => { it("does not expose runtime modules", () => { const forbidden = [ @@ -104,4 +171,71 @@ describe("plugin-sdk exports", () => { expect(sdk).toHaveProperty(key); } }); + + it("emits importable bundled subpath entries", { timeout: 240_000 }, async () => { + const outDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-plugin-sdk-build-")); + const fixtureDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-plugin-sdk-consumer-")); + + try { + await build({ + clean: true, + config: false, + dts: false, + entry: Object.fromEntries( + pluginSdkEntrypoints.map((entry) => [entry, `src/plugin-sdk/${entry}.ts`]), + ), + env: { NODE_ENV: "production" }, + fixedExtension: false, + logLevel: "error", + outDir, + platform: "node", + }); + + for (const entry of pluginSdkEntrypoints) { + const module = await import(pathToFileURL(path.join(outDir, `${entry}.js`)).href); + expect(module).toBeTypeOf("object"); + } + + const packageDir = path.join(fixtureDir, "openclaw"); + const consumerDir = path.join(fixtureDir, "consumer"); + const consumerEntry = path.join(consumerDir, "import-plugin-sdk.mjs"); + + await fs.mkdir(path.join(packageDir, "dist"), { recursive: true }); + await fs.symlink(outDir, path.join(packageDir, "dist", "plugin-sdk"), "dir"); + await fs.writeFile( + path.join(packageDir, "package.json"), + JSON.stringify( + { + exports: buildPluginSdkPackageExports(), + name: "openclaw", + type: "module", + }, + null, + 2, + ), + ); + + await fs.mkdir(path.join(consumerDir, "node_modules"), { recursive: true }); + await fs.symlink(packageDir, path.join(consumerDir, "node_modules", "openclaw"), "dir"); + await fs.writeFile( + consumerEntry, + [ + `const specifiers = ${JSON.stringify(pluginSdkSpecifiers)};`, + "const results = {};", + "for (const specifier of specifiers) {", + " results[specifier] = typeof (await import(specifier));", + "}", + "export default results;", + ].join("\n"), + ); + + const { default: importResults } = await import(pathToFileURL(consumerEntry).href); + expect(importResults).toEqual( + Object.fromEntries(pluginSdkSpecifiers.map((specifier) => [specifier, "object"])), + ); + } finally { + await fs.rm(outDir, { recursive: true, force: true }); + await fs.rm(fixtureDir, { recursive: true, force: true }); + } + }); }); diff --git a/src/shared/assistant-identity-values.test.ts b/src/shared/assistant-identity-values.test.ts index f0e594cc7e7..ef3440ee2a0 100644 --- a/src/shared/assistant-identity-values.test.ts +++ b/src/shared/assistant-identity-values.test.ts @@ -10,6 +10,7 @@ describe("shared/assistant-identity-values", () => { it("trims values and preserves strings within the limit", () => { expect(coerceIdentityValue(" OpenClaw ", 20)).toBe("OpenClaw"); + expect(coerceIdentityValue(" OpenClaw ", 8)).toBe("OpenClaw"); }); it("truncates overlong trimmed values at the exact limit", () => { @@ -18,5 +19,6 @@ describe("shared/assistant-identity-values", () => { it("returns an empty string when truncating to a zero-length limit", () => { expect(coerceIdentityValue(" OpenClaw ", 0)).toBe(""); + expect(coerceIdentityValue(" OpenClaw ", -1)).toBe("OpenCla"); }); }); diff --git a/src/shared/avatar-policy.test.ts b/src/shared/avatar-policy.test.ts index cbc345767e7..9f2dadeca0c 100644 --- a/src/shared/avatar-policy.test.ts +++ b/src/shared/avatar-policy.test.ts @@ -39,11 +39,13 @@ describe("avatar policy", () => { expect(isPathWithinRoot(root, root)).toBe(true); expect(isPathWithinRoot(root, path.resolve("/tmp/root/avatars/a.png"))).toBe(true); expect(isPathWithinRoot(root, path.resolve("/tmp/root/../outside.png"))).toBe(false); + expect(isPathWithinRoot(root, path.resolve("/tmp/root-sibling/avatar.png"))).toBe(false); }); it("detects avatar-like path strings", () => { expect(looksLikeAvatarPath("avatars/openclaw.svg")).toBe(true); expect(looksLikeAvatarPath("openclaw.webp")).toBe(true); + expect(looksLikeAvatarPath("avatar.ico")).toBe(true); expect(looksLikeAvatarPath("A")).toBe(false); }); diff --git a/src/shared/chat-envelope.test.ts b/src/shared/chat-envelope.test.ts index 0bd513c1b61..ca214f37316 100644 --- a/src/shared/chat-envelope.test.ts +++ b/src/shared/chat-envelope.test.ts @@ -14,6 +14,7 @@ describe("shared/chat-envelope", () => { expect(stripEnvelope("hello")).toBe("hello"); expect(stripEnvelope("[note] hello")).toBe("[note] hello"); expect(stripEnvelope("[2026/01/24 13:36] hello")).toBe("[2026/01/24 13:36] hello"); + expect(stripEnvelope("[Teams] hello")).toBe("[Teams] hello"); }); it("removes standalone message id hint lines but keeps inline mentions", () => { @@ -21,6 +22,7 @@ describe("shared/chat-envelope", () => { expect(stripMessageIdHints("hello\n [message_id: abc123] \nworld")).toBe("hello\nworld"); expect(stripMessageIdHints("[message_id: abc123]\nhello")).toBe("hello"); expect(stripMessageIdHints("[message_id: abc123]")).toBe(""); + expect(stripMessageIdHints("hello\r\n[MESSAGE_ID: abc123]\r\nworld")).toBe("hello\nworld"); expect(stripMessageIdHints("I typed [message_id: abc123] inline")).toBe( "I typed [message_id: abc123] inline", ); diff --git a/src/shared/chat-envelope.ts b/src/shared/chat-envelope.ts index 409a41357a1..b6bb1457a96 100644 --- a/src/shared/chat-envelope.ts +++ b/src/shared/chat-envelope.ts @@ -39,7 +39,7 @@ export function stripEnvelope(text: string): string { } export function stripMessageIdHints(text: string): string { - if (!text.includes("[message_id:")) { + if (!/\[message_id:/i.test(text)) { return text; } const lines = text.split(/\r?\n/); diff --git a/src/shared/config-eval.test.ts b/src/shared/config-eval.test.ts index 48ddb9e3298..7891c17142c 100644 --- a/src/shared/config-eval.test.ts +++ b/src/shared/config-eval.test.ts @@ -1,12 +1,40 @@ -import { describe, expect, it } from "vitest"; +import fs from "node:fs"; +import path from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { evaluateRuntimeEligibility, evaluateRuntimeRequires, + hasBinary, isConfigPathTruthyWithDefaults, isTruthy, resolveConfigPath, + resolveRuntimePlatform, } from "./config-eval.js"; +const originalPlatformDescriptor = Object.getOwnPropertyDescriptor(process, "platform"); +const originalPath = process.env.PATH; +const originalPathExt = process.env.PATHEXT; + +function setPlatform(platform: NodeJS.Platform): void { + Object.defineProperty(process, "platform", { + value: platform, + configurable: true, + }); +} + +afterEach(() => { + vi.restoreAllMocks(); + process.env.PATH = originalPath; + if (originalPathExt === undefined) { + delete process.env.PATHEXT; + } else { + process.env.PATHEXT = originalPathExt; + } + if (originalPlatformDescriptor) { + Object.defineProperty(process, "platform", originalPlatformDescriptor); + } +}); + describe("config-eval helpers", () => { it("normalizes truthy values across primitive types", () => { expect(isTruthy(undefined)).toBe(false); @@ -51,6 +79,53 @@ describe("config-eval helpers", () => { ).toBe(true); expect(isConfigPathTruthyWithDefaults(config, "browser.other", {})).toBe(false); }); + + it("returns the active runtime platform", () => { + setPlatform("darwin"); + expect(resolveRuntimePlatform()).toBe("darwin"); + }); + + it("caches binary lookups until PATH changes", () => { + process.env.PATH = ["/missing/bin", "/found/bin"].join(path.delimiter); + const accessSpy = vi.spyOn(fs, "accessSync").mockImplementation((candidate) => { + if (String(candidate) === path.join("/found/bin", "tool")) { + return undefined; + } + throw new Error("missing"); + }); + + expect(hasBinary("tool")).toBe(true); + expect(hasBinary("tool")).toBe(true); + expect(accessSpy).toHaveBeenCalledTimes(2); + + process.env.PATH = "/other/bin"; + accessSpy.mockClear(); + accessSpy.mockImplementation(() => { + throw new Error("missing"); + }); + + expect(hasBinary("tool")).toBe(false); + expect(accessSpy).toHaveBeenCalledTimes(1); + }); + + it("checks PATHEXT candidates on Windows", () => { + setPlatform("win32"); + process.env.PATH = "/tools"; + process.env.PATHEXT = ".EXE;.CMD"; + const accessSpy = vi.spyOn(fs, "accessSync").mockImplementation((candidate) => { + if (String(candidate) === "/tools/tool.CMD") { + return undefined; + } + throw new Error("missing"); + }); + + expect(hasBinary("tool")).toBe(true); + expect(accessSpy.mock.calls.map(([candidate]) => String(candidate))).toEqual([ + "/tools/tool", + "/tools/tool.EXE", + "/tools/tool.CMD", + ]); + }); }); describe("evaluateRuntimeRequires", () => { diff --git a/src/shared/device-auth-store.test.ts b/src/shared/device-auth-store.test.ts index be070ee79cd..b0346afff9a 100644 --- a/src/shared/device-auth-store.test.ts +++ b/src/shared/device-auth-store.test.ts @@ -50,6 +50,36 @@ describe("device-auth-store", () => { ).toBeNull(); }); + it("returns null for missing stores and malformed token entries", () => { + expect( + loadDeviceAuthTokenFromStore({ + adapter: createAdapter().adapter, + deviceId: "device-1", + role: "operator", + }), + ).toBeNull(); + + const { adapter } = createAdapter({ + version: 1, + deviceId: "device-1", + tokens: { + operator: { + token: 123 as unknown as string, + role: "operator", + scopes: [], + updatedAtMs: 1, + }, + }, + }); + expect( + loadDeviceAuthTokenFromStore({ + adapter, + deviceId: "device-1", + role: "operator", + }), + ).toBeNull(); + }); + it("stores normalized roles and deduped sorted scopes while preserving same-device tokens", () => { vi.spyOn(Date, "now").mockReturnValue(1234); const { adapter, writes, readStore } = createAdapter({ @@ -130,6 +160,44 @@ describe("device-auth-store", () => { }); }); + it("overwrites existing entries for the same normalized role", () => { + vi.spyOn(Date, "now").mockReturnValue(2222); + const { adapter, readStore } = createAdapter({ + version: 1, + deviceId: "device-1", + tokens: { + operator: { + token: "old-token", + role: "operator", + scopes: ["operator.read"], + updatedAtMs: 10, + }, + }, + }); + + const entry = storeDeviceAuthTokenInStore({ + adapter, + deviceId: "device-1", + role: " operator ", + token: "new-token", + scopes: ["operator.write"], + }); + + expect(entry).toEqual({ + token: "new-token", + role: "operator", + scopes: ["operator.write"], + updatedAtMs: 2222, + }); + expect(readStore()).toEqual({ + version: 1, + deviceId: "device-1", + tokens: { + operator: entry, + }, + }); + }); + it("avoids writes when clearing missing roles or mismatched devices", () => { const missingRole = createAdapter({ version: 1, diff --git a/src/shared/device-auth.test.ts b/src/shared/device-auth.test.ts index a3bc6fa3956..d3018f5ba0a 100644 --- a/src/shared/device-auth.test.ts +++ b/src/shared/device-auth.test.ts @@ -13,6 +13,7 @@ describe("shared/device-auth", () => { normalizeDeviceAuthScopes([" node.invoke ", "operator.read", "", "node.invoke", "a.scope"]), ).toEqual(["a.scope", "node.invoke", "operator.read"]); expect(normalizeDeviceAuthScopes(undefined)).toEqual([]); + expect(normalizeDeviceAuthScopes(null as unknown as string[])).toEqual([]); expect(normalizeDeviceAuthScopes([" ", "\t", "\n"])).toEqual([]); expect(normalizeDeviceAuthScopes(["z.scope", "A.scope", "m.scope"])).toEqual([ "A.scope", diff --git a/src/shared/entry-metadata.test.ts b/src/shared/entry-metadata.test.ts index cf94453a62e..ea52cd033e4 100644 --- a/src/shared/entry-metadata.test.ts +++ b/src/shared/entry-metadata.test.ts @@ -46,4 +46,16 @@ describe("shared/entry-metadata", () => { homepage: "https://openclaw.ai/install", }); }); + + it("does not fall back once frontmatter homepage aliases are present but blank", () => { + expect( + resolveEmojiAndHomepage({ + frontmatter: { + homepage: " ", + website: "https://docs.openclaw.ai", + url: "https://openclaw.ai/install", + }, + }), + ).toEqual({}); + }); }); diff --git a/src/shared/entry-status.test.ts b/src/shared/entry-status.test.ts index 88913913011..68cce75c982 100644 --- a/src/shared/entry-status.test.ts +++ b/src/shared/entry-status.test.ts @@ -129,4 +129,33 @@ describe("shared/entry-status", () => { configChecks: [], }); }); + + it("returns empty requirements when metadata and frontmatter are missing", () => { + const result = evaluateEntryMetadataRequirements({ + always: false, + hasLocalBin: () => false, + localPlatform: "linux", + isEnvSatisfied: () => false, + isConfigSatisfied: () => false, + }); + + expect(result).toEqual({ + required: { + bins: [], + anyBins: [], + env: [], + config: [], + os: [], + }, + missing: { + bins: [], + anyBins: [], + env: [], + config: [], + os: [], + }, + requirementsSatisfied: true, + configChecks: [], + }); + }); }); diff --git a/src/shared/frontmatter.test.ts b/src/shared/frontmatter.test.ts index 94cd4acabef..69d48e05b57 100644 --- a/src/shared/frontmatter.test.ts +++ b/src/shared/frontmatter.test.ts @@ -27,6 +27,7 @@ describe("shared/frontmatter", () => { expect(parseFrontmatterBool("true", false)).toBe(true); expect(parseFrontmatterBool("false", true)).toBe(false); expect(parseFrontmatterBool(undefined, true)).toBe(true); + expect(parseFrontmatterBool("maybe", false)).toBe(false); }); test("resolveOpenClawManifestBlock reads current manifest keys and custom metadata fields", () => { @@ -53,6 +54,8 @@ describe("shared/frontmatter", () => { expect( resolveOpenClawManifestBlock({ frontmatter: { metadata: "not-json5" } }), ).toBeUndefined(); + expect(resolveOpenClawManifestBlock({ frontmatter: { metadata: "123" } })).toBeUndefined(); + expect(resolveOpenClawManifestBlock({ frontmatter: { metadata: "[]" } })).toBeUndefined(); expect( resolveOpenClawManifestBlock({ frontmatter: { metadata: "{ nope: { a: 1 } }" } }), ).toBeUndefined(); @@ -120,6 +123,40 @@ describe("shared/frontmatter", () => { }); }); + it("prefers explicit kind, ignores invalid common fields, and leaves missing ones untouched", () => { + const parsed = parseOpenClawManifestInstallBase( + { + kind: " npm ", + type: "brew", + id: 42, + label: null, + bins: [" ", ""], + }, + ["brew", "npm"], + ); + + expect(parsed).toEqual({ + raw: { + kind: " npm ", + type: "brew", + id: 42, + label: null, + bins: [" ", ""], + }, + kind: "npm", + }); + expect( + applyOpenClawManifestInstallCommonFields( + { id: "keep", label: "Keep", bins: ["bun"] }, + parsed!, + ), + ).toEqual({ + id: "keep", + label: "Keep", + bins: ["bun"], + }); + }); + it("maps install entries through the parser and filters rejected specs", () => { expect( resolveOpenClawManifestInstall( diff --git a/src/shared/gateway-bind-url.test.ts b/src/shared/gateway-bind-url.test.ts index 23dd855c4e6..5bf9c8582a5 100644 --- a/src/shared/gateway-bind-url.test.ts +++ b/src/shared/gateway-bind-url.test.ts @@ -3,25 +3,33 @@ import { resolveGatewayBindUrl } from "./gateway-bind-url.js"; describe("shared/gateway-bind-url", () => { it("returns null for loopback/default binds", () => { + const pickTailnetHost = vi.fn(() => "100.64.0.1"); + const pickLanHost = vi.fn(() => "192.168.1.2"); + expect( resolveGatewayBindUrl({ scheme: "ws", port: 18789, - pickTailnetHost: () => "100.64.0.1", - pickLanHost: () => "192.168.1.2", + pickTailnetHost, + pickLanHost, }), ).toBeNull(); + expect(pickTailnetHost).not.toHaveBeenCalled(); + expect(pickLanHost).not.toHaveBeenCalled(); }); it("resolves custom binds only when custom host is present after trimming", () => { + const pickTailnetHost = vi.fn(); + const pickLanHost = vi.fn(); + expect( resolveGatewayBindUrl({ bind: "custom", customBindHost: " gateway.local ", scheme: "wss", port: 443, - pickTailnetHost: vi.fn(), - pickLanHost: vi.fn(), + pickTailnetHost, + pickLanHost, }), ).toEqual({ url: "wss://gateway.local:443", @@ -34,12 +42,14 @@ describe("shared/gateway-bind-url", () => { customBindHost: " ", scheme: "ws", port: 18789, - pickTailnetHost: vi.fn(), - pickLanHost: vi.fn(), + pickTailnetHost, + pickLanHost, }), ).toEqual({ error: "gateway.bind=custom requires gateway.customBindHost.", }); + expect(pickTailnetHost).not.toHaveBeenCalled(); + expect(pickLanHost).not.toHaveBeenCalled(); }); it("resolves tailnet and lan binds or returns clear errors", () => { @@ -91,4 +101,21 @@ describe("shared/gateway-bind-url", () => { error: "gateway.bind=lan set, but no private LAN IP was found.", }); }); + + it("returns null for unrecognized bind values without probing pickers", () => { + const pickTailnetHost = vi.fn(() => "100.64.0.1"); + const pickLanHost = vi.fn(() => "192.168.1.2"); + + expect( + resolveGatewayBindUrl({ + bind: "loopbackish", + scheme: "ws", + port: 18789, + pickTailnetHost, + pickLanHost, + }), + ).toBeNull(); + expect(pickTailnetHost).not.toHaveBeenCalled(); + expect(pickLanHost).not.toHaveBeenCalled(); + }); }); diff --git a/src/shared/model-param-b.test.ts b/src/shared/model-param-b.test.ts index 7fb9a7b82d4..8bbafe3f529 100644 --- a/src/shared/model-param-b.test.ts +++ b/src/shared/model-param-b.test.ts @@ -6,6 +6,7 @@ describe("shared/model-param-b", () => { expect(inferParamBFromIdOrName("llama-8b mixtral-22b")).toBe(22); expect(inferParamBFromIdOrName("Qwen 0.5B Instruct")).toBe(0.5); expect(inferParamBFromIdOrName("prefix M7B and q4_32b")).toBe(32); + expect(inferParamBFromIdOrName("(70b) + m1.5b + qwen-14b")).toBe(70); }); it("ignores malformed, zero, and non-delimited matches", () => { @@ -13,5 +14,6 @@ describe("shared/model-param-b", () => { expect(inferParamBFromIdOrName("model 0b")).toBeNull(); expect(inferParamBFromIdOrName("model b5")).toBeNull(); expect(inferParamBFromIdOrName("foo70bbar")).toBeNull(); + expect(inferParamBFromIdOrName("ab7b model")).toBeNull(); }); }); diff --git a/src/shared/net/ipv4.test.ts b/src/shared/net/ipv4.test.ts index 21ff99b982b..40dd024138f 100644 --- a/src/shared/net/ipv4.test.ts +++ b/src/shared/net/ipv4.test.ts @@ -13,12 +13,16 @@ describe("shared/net/ipv4", () => { }); it("accepts canonical dotted-decimal ipv4 only", () => { + expect(validateDottedDecimalIPv4Input("0.0.0.0")).toBeUndefined(); expect(validateDottedDecimalIPv4Input("192.168.1.100")).toBeUndefined(); expect(validateDottedDecimalIPv4Input(" 192.168.1.100 ")).toBeUndefined(); expect(validateDottedDecimalIPv4Input("0177.0.0.1")).toBe( "Invalid IPv4 address (e.g., 192.168.1.100)", ); expect(validateDottedDecimalIPv4Input("[192.168.1.100]")).toBeUndefined(); + expect(validateDottedDecimalIPv4Input("127.1")).toBe( + "Invalid IPv4 address (e.g., 192.168.1.100)", + ); expect(validateDottedDecimalIPv4Input("example.com")).toBe( "Invalid IPv4 address (e.g., 192.168.1.100)", ); diff --git a/src/shared/operator-scope-compat.test.ts b/src/shared/operator-scope-compat.test.ts index e48a17ad398..44236ca7341 100644 --- a/src/shared/operator-scope-compat.test.ts +++ b/src/shared/operator-scope-compat.test.ts @@ -2,6 +2,16 @@ import { describe, expect, it } from "vitest"; import { roleScopesAllow } from "./operator-scope-compat.js"; describe("roleScopesAllow", () => { + it("allows empty requested scope lists regardless of granted scopes", () => { + expect( + roleScopesAllow({ + role: "operator", + requestedScopes: [], + allowedScopes: [], + }), + ).toBe(true); + }); + it("treats operator.read as satisfied by read/write/admin scopes", () => { expect( roleScopesAllow({ @@ -85,6 +95,13 @@ describe("roleScopesAllow", () => { allowedScopes: ["operator.admin"], }), ).toBe(false); + expect( + roleScopesAllow({ + role: " node ", + requestedScopes: [" system.run ", "system.run", " "], + allowedScopes: ["system.run", "operator.admin"], + }), + ).toBe(true); }); it("normalizes blank and duplicate scopes before evaluating", () => { diff --git a/src/shared/requirements.test.ts b/src/shared/requirements.test.ts index 0a05a0eb85c..3b47e768f2c 100644 --- a/src/shared/requirements.test.ts +++ b/src/shared/requirements.test.ts @@ -22,6 +22,12 @@ describe("requirements helpers", () => { }); it("resolveMissingAnyBins requires at least one", () => { + expect( + resolveMissingAnyBins({ + required: [], + hasLocalBin: () => false, + }), + ).toEqual([]); expect( resolveMissingAnyBins({ required: ["a", "b"], @@ -38,6 +44,8 @@ describe("requirements helpers", () => { }); it("resolveMissingOs allows remote platform", () => { + expect(resolveMissingOs({ required: [], localPlatform: "linux" })).toEqual([]); + expect(resolveMissingOs({ required: ["linux"], localPlatform: "linux" })).toEqual([]); expect( resolveMissingOs({ required: ["darwin"], @@ -164,4 +172,31 @@ describe("requirements helpers", () => { expect(res.missing).toEqual({ bins: [], anyBins: [], env: [], config: [], os: [] }); expect(res.eligible).toBe(true); }); + + it("evaluateRequirementsFromMetadata defaults missing metadata to empty requirements", () => { + const res = evaluateRequirementsFromMetadata({ + always: false, + hasLocalBin: () => false, + localPlatform: "linux", + isEnvSatisfied: () => false, + isConfigSatisfied: () => false, + }); + + expect(res.required).toEqual({ + bins: [], + anyBins: [], + env: [], + config: [], + os: [], + }); + expect(res.missing).toEqual({ + bins: [], + anyBins: [], + env: [], + config: [], + os: [], + }); + expect(res.configChecks).toEqual([]); + expect(res.eligible).toBe(true); + }); }); diff --git a/src/shared/tailscale-status.test.ts b/src/shared/tailscale-status.test.ts index 94128e700ed..ebd89629a4c 100644 --- a/src/shared/tailscale-status.test.ts +++ b/src/shared/tailscale-status.test.ts @@ -54,9 +54,23 @@ describe("shared/tailscale-status", () => { expect(run).toHaveBeenCalledTimes(2); }); + it("continues when the first candidate returns success but malformed Self data", async () => { + const run = vi + .fn() + .mockResolvedValueOnce({ code: 0, stdout: '{"Self":"bad"}' }) + .mockResolvedValueOnce({ + code: 0, + stdout: 'prefix {"Self":{"TailscaleIPs":["100.64.0.11"]}} suffix', + }); + + await expect(resolveTailnetHostWithRunner(run)).resolves.toBe("100.64.0.11"); + expect(run).toHaveBeenCalledTimes(2); + }); + it("returns null for non-zero exits, blank output, or invalid json", async () => { const run = vi .fn() + .mockResolvedValueOnce({ code: null, stdout: "boom" }) .mockResolvedValueOnce({ code: 1, stdout: "boom" }) .mockResolvedValueOnce({ code: 0, stdout: " " }); diff --git a/src/shared/text/reasoning-tags.test.ts b/src/shared/text/reasoning-tags.test.ts index 40cd133beac..86180e21a3f 100644 --- a/src/shared/text/reasoning-tags.test.ts +++ b/src/shared/text/reasoning-tags.test.ts @@ -146,6 +146,10 @@ describe("stripReasoningTagsFromText", () => { input: "`` in code, visible outside", expected: "`` in code, visible outside", }, + { + input: "A visible B", + expected: "A visible B", + }, ] as const; for (const { input, expected } of cases) { expect(stripReasoningTagsFromText(input)).toBe(expected); @@ -195,6 +199,12 @@ describe("stripReasoningTagsFromText", () => { expect(stripReasoningTagsFromText(input, { mode })).toBe(expected); } }); + + it("still strips fully closed reasoning blocks in preserve mode", () => { + expect(stripReasoningTagsFromText("A hidden B", { mode: "preserve" })).toBe( + "A B", + ); + }); }); describe("trim options", () => { @@ -221,4 +231,10 @@ describe("stripReasoningTagsFromText", () => { } }); }); + + it("does not leak regex state across repeated calls", () => { + expect(stripReasoningTagsFromText("A 1 B")).toBe("A 1 B"); + expect(stripReasoningTagsFromText("C 2 D")).toBe("C 2 D"); + expect(stripReasoningTagsFromText("E x F")).toBe("E F"); + }); }); diff --git a/src/shared/usage-aggregates.test.ts b/src/shared/usage-aggregates.test.ts index e5ba960ad95..dc6896b7490 100644 --- a/src/shared/usage-aggregates.test.ts +++ b/src/shared/usage-aggregates.test.ts @@ -16,6 +16,13 @@ describe("shared/usage-aggregates", () => { }; mergeUsageLatency(totals, undefined); + mergeUsageLatency(totals, { + count: 0, + avgMs: 999, + minMs: 1, + maxMs: 999, + p95Ms: 999, + }); mergeUsageLatency(totals, { count: 2, avgMs: 50, @@ -51,6 +58,7 @@ describe("shared/usage-aggregates", () => { { date: "2026-03-12", count: 1, avgMs: 120, minMs: 120, maxMs: 120, p95Ms: 120 }, { date: "2026-03-11", count: 1, avgMs: 30, minMs: 30, maxMs: 30, p95Ms: 30 }, ]); + mergeUsageDailyLatency(dailyLatencyMap, null); const tail = buildUsageAggregateTail({ byChannelMap: new Map([ @@ -114,4 +122,38 @@ describe("shared/usage-aggregates", () => { expect(tail.latency).toBeUndefined(); expect(tail.dailyLatency).toEqual([]); }); + + it("normalizes zero-count daily latency entries to zero averages and mins", () => { + const dailyLatencyMap = new Map([ + [ + "2026-03-12", + { + date: "2026-03-12", + count: 0, + sum: 0, + min: Number.POSITIVE_INFINITY, + max: 0, + p95Max: 0, + }, + ], + ]); + + const tail = buildUsageAggregateTail({ + byChannelMap: new Map(), + latencyTotals: { + count: 0, + sum: 0, + min: Number.POSITIVE_INFINITY, + max: 0, + p95Max: 0, + }, + dailyLatencyMap, + modelDailyMap: new Map(), + dailyMap: new Map(), + }); + + expect(tail.dailyLatency).toEqual([ + { date: "2026-03-12", count: 0, avgMs: 0, minMs: 0, maxMs: 0, p95Ms: 0 }, + ]); + }); }); diff --git a/src/telegram/webhook.test.ts b/src/telegram/webhook.test.ts index 1b630b034df..0f2736a30b9 100644 --- a/src/telegram/webhook.test.ts +++ b/src/telegram/webhook.test.ts @@ -88,6 +88,70 @@ async function postWebhookJson(params: { ); } +async function postWebhookHeadersOnly(params: { + port: number; + path: string; + declaredLength: number; + secret?: string; + timeoutMs?: number; +}): Promise<{ statusCode: number; body: string }> { + return await new Promise((resolve, reject) => { + let settled = false; + const finishResolve = (value: { statusCode: number; body: string }) => { + if (settled) { + return; + } + settled = true; + clearTimeout(timeout); + resolve(value); + }; + const finishReject = (error: unknown) => { + if (settled) { + return; + } + settled = true; + clearTimeout(timeout); + reject(error); + }; + + const req = request( + { + hostname: "127.0.0.1", + port: params.port, + path: params.path, + method: "POST", + headers: { + "content-type": "application/json", + "content-length": String(params.declaredLength), + ...(params.secret ? { "x-telegram-bot-api-secret-token": params.secret } : {}), + }, + }, + (res) => { + collectResponseBody(res, (payload) => { + finishResolve(payload); + req.destroy(); + }); + }, + ); + + const timeout = setTimeout(() => { + req.destroy( + new Error(`webhook header-only post timed out after ${params.timeoutMs ?? 5_000}ms`), + ); + finishReject(new Error("timed out waiting for webhook response")); + }, params.timeoutMs ?? 5_000); + + req.on("error", (error) => { + if (settled && (error as NodeJS.ErrnoException).code === "ECONNRESET") { + return; + } + finishReject(error); + }); + + req.flushHeaders(); + }); +} + function createDeterministicRng(seed: number): () => number { let state = seed >>> 0; return () => { @@ -399,7 +463,34 @@ describe("startTelegramWebhook", () => { secret: TELEGRAM_SECRET, }); expect(response.status).toBe(200); - expect(handlerSpy).toHaveBeenCalled(); + expect(handlerSpy).toHaveBeenCalledWith( + JSON.parse(payload), + expect.any(Function), + TELEGRAM_SECRET, + expect.any(Function), + ); + }, + ); + }); + + it("rejects unauthenticated requests before reading the request body", async () => { + handlerSpy.mockClear(); + await withStartedWebhook( + { + secret: TELEGRAM_SECRET, + path: TELEGRAM_WEBHOOK_PATH, + }, + async ({ port }) => { + const response = await postWebhookHeadersOnly({ + port, + path: TELEGRAM_WEBHOOK_PATH, + declaredLength: 1_024 * 1_024, + secret: "wrong-secret", + }); + + expect(response.statusCode).toBe(401); + expect(response.body).toBe("unauthorized"); + expect(handlerSpy).not.toHaveBeenCalled(); }, ); }); diff --git a/src/telegram/webhook.ts b/src/telegram/webhook.ts index 1de38b1bb36..c049089a2ad 100644 --- a/src/telegram/webhook.ts +++ b/src/telegram/webhook.ts @@ -1,3 +1,4 @@ +import { timingSafeEqual } from "node:crypto"; import { createServer } from "node:http"; import { InputFile, webhookCallback } from "grammy"; import type { OpenClawConfig } from "../config/config.js"; @@ -74,6 +75,28 @@ async function initializeTelegramWebhookBot(params: { }); } +function resolveSingleHeaderValue(header: string | string[] | undefined): string | undefined { + if (typeof header === "string") { + return header; + } + if (Array.isArray(header) && header.length === 1) { + return header[0]; + } + return undefined; +} + +function hasValidTelegramWebhookSecret( + secretHeader: string | undefined, + expectedSecret: string, +): boolean { + if (typeof secretHeader !== "string") { + return false; + } + const actual = Buffer.from(secretHeader, "utf-8"); + const expected = Buffer.from(expectedSecret, "utf-8"); + return actual.length === expected.length && timingSafeEqual(actual, expected); +} + export async function startTelegramWebhook(opts: { token: string; accountId?: string; @@ -147,6 +170,13 @@ export async function startTelegramWebhook(opts: { if (diagnosticsEnabled) { logWebhookReceived({ channel: "telegram", updateType: "telegram-post" }); } + const secretHeader = resolveSingleHeaderValue(req.headers["x-telegram-bot-api-secret-token"]); + if (!hasValidTelegramWebhookSecret(secretHeader, secret)) { + res.shouldKeepAlive = false; + res.setHeader("Connection", "close"); + respondText(401, "unauthorized"); + return; + } void (async () => { const body = await readJsonBodyWithLimit(req, { maxBytes: TELEGRAM_WEBHOOK_MAX_BODY_BYTES, @@ -189,8 +219,6 @@ export async function startTelegramWebhook(opts: { replied = true; respondText(401, "unauthorized"); }; - const secretHeaderRaw = req.headers["x-telegram-bot-api-secret-token"]; - const secretHeader = Array.isArray(secretHeaderRaw) ? secretHeaderRaw[0] : secretHeaderRaw; await handler(body.value, reply, secretHeader, unauthorized); if (!replied) { diff --git a/tsdown.config.ts b/tsdown.config.ts index 80833de2a14..1806debd474 100644 --- a/tsdown.config.ts +++ b/tsdown.config.ts @@ -116,12 +116,12 @@ export default defineConfig([ "line/template-messages": "src/line/template-messages.ts", }, }), - ...pluginSdkEntrypoints.map((entry) => - nodeBuildConfig({ - entry: `src/plugin-sdk/${entry}.ts`, - outDir: "dist/plugin-sdk", - }), - ), + nodeBuildConfig({ + // Bundle all plugin-sdk entries in a single build so the bundler can share + // common chunks instead of duplicating them per entry (~712MB heap saved). + entry: Object.fromEntries(pluginSdkEntrypoints.map((e) => [e, `src/plugin-sdk/${e}.ts`])), + outDir: "dist/plugin-sdk", + }), nodeBuildConfig({ entry: "src/extensionAPI.ts", }),