From 9b590c9f671e2effc4843d078004a278bc0c0c7b Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:47:33 +0000 Subject: [PATCH 01/41] test: tighten shared reasoning tag coverage --- src/shared/text/reasoning-tags.test.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) 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"); + }); }); From daca6c9df202b5b14d41992983ad31591759aafc Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:48:40 +0000 Subject: [PATCH 02/41] test: tighten small shared helper coverage --- src/shared/assistant-identity-values.test.ts | 2 ++ src/shared/model-param-b.test.ts | 2 ++ src/shared/net/ipv4.test.ts | 4 ++++ 3 files changed, 8 insertions(+) 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/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)", ); From fbcea506ba43406b7f3fa4b7d6818060c28051e2 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:49:50 +0000 Subject: [PATCH 03/41] test: tighten shared gateway bind and avatar coverage --- src/shared/avatar-policy.test.ts | 2 ++ src/shared/gateway-bind-url.test.ts | 39 ++++++++++++++++++++++++----- 2 files changed, 35 insertions(+), 6 deletions(-) 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/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(); + }); }); From e665888a45d933c229aefe73c2a05e125290b983 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:51:01 +0000 Subject: [PATCH 04/41] test: tighten shared usage aggregate coverage --- src/shared/usage-aggregates.test.ts | 42 +++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) 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 }, + ]); + }); }); From 4de268587cb649ecd7c9e82f46351d3c20b3fb59 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:52:01 +0000 Subject: [PATCH 05/41] test: tighten shared tailscale fallback coverage --- src/shared/tailscale-status.test.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) 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: " " }); From 52900b48ad375cf1c2e976e546164eca6b920123 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:53:11 +0000 Subject: [PATCH 06/41] test: tighten shared policy helper coverage --- src/shared/device-auth.test.ts | 1 + src/shared/operator-scope-compat.test.ts | 17 +++++++++++++++++ 2 files changed, 18 insertions(+) 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/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", () => { From 158d970e2b2c0f7dbb91490e254f16afe7fe2370 Mon Sep 17 00:00:00 2001 From: Val Alexander <68980965+BunsDev@users.noreply.github.com> Date: Fri, 13 Mar 2026 16:53:40 -0500 Subject: [PATCH 07/41] [codex] Polish sidebar status, agent skills, and chat rendering (#45451) * style: update chat layout and spacing for improved UI consistency - Adjusted margin and padding for .chat-thread and .content--chat to enhance layout. - Consolidated CSS selectors for better readability and maintainability. - Introduced new test for log parsing functionality to ensure accurate message extraction. * UI: polish agent skills, chat images, and sidebar status * test: stabilize vitest helper export types * UI: address review feedback on agents refresh and chat styles * test: update outbound gateway client fixture values * test: narrow shared ip fixtures to IPv4 --- src/cli/daemon-cli/lifecycle-core.test.ts | 2 + .../test-helpers/lifecycle-core-harness.ts | 36 +++- .../test-helpers/schtasks-base-mocks.ts | 4 +- src/daemon/test-helpers/schtasks-fixtures.ts | 8 +- .../outbound/outbound-send-service.test.ts | 9 +- src/shared/net/ip.test.ts | 3 +- .../bot-native-commands.test-helpers.ts | 28 ++- ui/src/i18n/locales/de.ts | 1 + ui/src/i18n/locales/en.ts | 1 + ui/src/i18n/locales/es.ts | 1 + ui/src/i18n/locales/pt-BR.ts | 1 + ui/src/i18n/locales/zh-CN.ts | 1 + ui/src/i18n/locales/zh-TW.ts | 1 + ui/src/styles/chat/layout.css | 28 ++- ui/src/styles/chat/sidebar.css | 12 ++ ui/src/styles/chat/text.css | 13 ++ ui/src/styles/components.css | 4 +- ui/src/styles/layout.css | 45 +++-- ui/src/styles/layout.mobile.css | 8 +- ui/src/ui/app-render.helpers.ts | 17 ++ ui/src/ui/app-render.ts | 20 +- ui/src/ui/controllers/logs.test.ts | 28 +++ ui/src/ui/controllers/logs.ts | 2 + ui/src/ui/markdown.test.ts | 1 + ui/src/ui/markdown.ts | 2 +- ui/src/ui/sidebar-status.browser.test.ts | 24 +++ ui/src/ui/views/agents.test.ts | 174 ++++++++++++++++++ ui/src/ui/views/agents.ts | 6 +- 28 files changed, 427 insertions(+), 53 deletions(-) create mode 100644 ui/src/ui/controllers/logs.test.ts create mode 100644 ui/src/ui/sidebar-status.browser.test.ts create mode 100644 ui/src/ui/views/agents.test.ts diff --git a/src/cli/daemon-cli/lifecycle-core.test.ts b/src/cli/daemon-cli/lifecycle-core.test.ts index 7503e21ae5e..6e86ad0d23a 100644 --- a/src/cli/daemon-cli/lifecycle-core.test.ts +++ b/src/cli/daemon-cli/lifecycle-core.test.ts @@ -46,6 +46,7 @@ describe("runServiceRestart token drift", () => { }); resetLifecycleServiceMocks(); service.readCommand.mockResolvedValue({ + programArguments: [], environment: { OPENCLAW_GATEWAY_TOKEN: "service-token" }, }); stubEmptyGatewayEnv(); @@ -77,6 +78,7 @@ describe("runServiceRestart token drift", () => { }, }); service.readCommand.mockResolvedValue({ + programArguments: [], environment: { OPENCLAW_GATEWAY_TOKEN: "env-token" }, }); vi.stubEnv("OPENCLAW_GATEWAY_TOKEN", "env-token"); diff --git a/src/cli/daemon-cli/test-helpers/lifecycle-core-harness.ts b/src/cli/daemon-cli/test-helpers/lifecycle-core-harness.ts index 8e91db61664..6e2a93d5633 100644 --- a/src/cli/daemon-cli/test-helpers/lifecycle-core-harness.ts +++ b/src/cli/daemon-cli/test-helpers/lifecycle-core-harness.ts @@ -1,16 +1,36 @@ import { vi } from "vitest"; +import type { GatewayService } from "../../../daemon/service.js"; +import type { RuntimeEnv } from "../../../runtime.js"; +import type { MockFn } from "../../../test-utils/vitest-mock-fn.js"; export const runtimeLogs: string[] = []; -export const defaultRuntime = { - log: (message: string) => runtimeLogs.push(message), - error: vi.fn(), - exit: (code: number) => { - throw new Error(`__exit__:${code}`); - }, +type LifecycleRuntimeHarness = RuntimeEnv & { + error: MockFn; + exit: MockFn; }; -export const service = { +type LifecycleServiceHarness = GatewayService & { + install: MockFn; + uninstall: MockFn; + stop: MockFn; + isLoaded: MockFn; + readCommand: MockFn; + readRuntime: MockFn; + restart: MockFn; +}; + +export const defaultRuntime: LifecycleRuntimeHarness = { + log: (...args: unknown[]) => { + runtimeLogs.push(args.map((arg) => String(arg)).join(" ")); + }, + error: vi.fn(), + exit: vi.fn((code: number) => { + throw new Error(`__exit__:${code}`); + }), +}; + +export const service: LifecycleServiceHarness = { label: "TestService", loadedText: "loaded", notLoadedText: "not loaded", @@ -32,7 +52,7 @@ export function resetLifecycleServiceMocks() { service.readCommand.mockClear(); service.restart.mockClear(); service.isLoaded.mockResolvedValue(true); - service.readCommand.mockResolvedValue({ environment: {} }); + service.readCommand.mockResolvedValue({ programArguments: [], environment: {} }); service.restart.mockResolvedValue({ outcome: "completed" }); } diff --git a/src/daemon/test-helpers/schtasks-base-mocks.ts b/src/daemon/test-helpers/schtasks-base-mocks.ts index 48933ecdd1c..e3f0f950482 100644 --- a/src/daemon/test-helpers/schtasks-base-mocks.ts +++ b/src/daemon/test-helpers/schtasks-base-mocks.ts @@ -14,9 +14,9 @@ vi.mock("../schtasks-exec.js", () => ({ })); vi.mock("../../infra/ports.js", () => ({ - inspectPortUsage: (...args: unknown[]) => inspectPortUsage(...args), + inspectPortUsage: (port: number) => inspectPortUsage(port), })); vi.mock("../../process/kill-tree.js", () => ({ - killProcessTree: (...args: unknown[]) => killProcessTree(...args), + killProcessTree: (pid: number, opts?: { graceMs?: number }) => killProcessTree(pid, opts), })); diff --git a/src/daemon/test-helpers/schtasks-fixtures.ts b/src/daemon/test-helpers/schtasks-fixtures.ts index a89d7a0eb2e..4762b7543eb 100644 --- a/src/daemon/test-helpers/schtasks-fixtures.ts +++ b/src/daemon/test-helpers/schtasks-fixtures.ts @@ -2,11 +2,15 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { vi } from "vitest"; +import type { PortUsage } from "../../infra/ports-types.js"; +import type { killProcessTree as killProcessTreeImpl } from "../../process/kill-tree.js"; +import type { MockFn } from "../../test-utils/vitest-mock-fn.js"; export const schtasksResponses: Array<{ code: number; stdout: string; stderr: string }> = []; export const schtasksCalls: string[][] = []; -export const inspectPortUsage = vi.fn(); -export const killProcessTree = vi.fn(); + +export const inspectPortUsage: MockFn<(port: number) => Promise> = vi.fn(); +export const killProcessTree: MockFn = vi.fn(); export async function withWindowsEnv( prefix: string, diff --git a/src/infra/outbound/outbound-send-service.test.ts b/src/infra/outbound/outbound-send-service.test.ts index 391abee8dda..ac144265753 100644 --- a/src/infra/outbound/outbound-send-service.test.ts +++ b/src/infra/outbound/outbound-send-service.test.ts @@ -1,4 +1,5 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; +import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../../utils/message-channel.js"; const mocks = vi.hoisted(() => ({ getDefaultMediaLocalRoots: vi.fn(() => []), @@ -204,8 +205,8 @@ describe("executeSendAction", () => { url: "http://127.0.0.1:18789", token: "tok", timeoutMs: 5000, - clientName: "gateway", - mode: "gateway", + clientName: GATEWAY_CLIENT_NAMES.GATEWAY_CLIENT, + mode: GATEWAY_CLIENT_MODES.BACKEND, }, }, to: "channel:123", @@ -296,8 +297,8 @@ describe("executeSendAction", () => { url: "http://127.0.0.1:18789", token: "tok", timeoutMs: 5000, - clientName: "gateway", - mode: "gateway", + clientName: GATEWAY_CLIENT_NAMES.GATEWAY_CLIENT, + mode: GATEWAY_CLIENT_MODES.BACKEND, }, }, to: "channel:123", diff --git a/src/shared/net/ip.test.ts b/src/shared/net/ip.test.ts index 0328c7c87f2..2322a106c9d 100644 --- a/src/shared/net/ip.test.ts +++ b/src/shared/net/ip.test.ts @@ -6,6 +6,7 @@ import { isCanonicalDottedDecimalIPv4, isCarrierGradeNatIpv4Address, isIpInCidr, + isIpv4Address, isIpv6Address, isLegacyIpv4Literal, isLoopbackIpAddress, @@ -88,7 +89,7 @@ describe("shared ip helpers", () => { expect(loopback?.kind()).toBe("ipv4"); expect(benchmark?.kind()).toBe("ipv4"); - if (!loopback || loopback.kind() !== "ipv4" || !benchmark || benchmark.kind() !== "ipv4") { + if (!loopback || !isIpv4Address(loopback) || !benchmark || !isIpv4Address(benchmark)) { throw new Error("expected ipv4 fixtures"); } diff --git a/src/telegram/bot-native-commands.test-helpers.ts b/src/telegram/bot-native-commands.test-helpers.ts index 02f1028becf..eef028c8315 100644 --- a/src/telegram/bot-native-commands.test-helpers.ts +++ b/src/telegram/bot-native-commands.test-helpers.ts @@ -3,12 +3,28 @@ import type { OpenClawConfig } from "../config/config.js"; import type { ChannelGroupPolicy } from "../config/group-policy.js"; import type { TelegramAccountConfig } from "../config/types.js"; import type { RuntimeEnv } from "../runtime.js"; +import type { MockFn } from "../test-utils/vitest-mock-fn.js"; import { registerTelegramNativeCommands } from "./bot-native-commands.js"; type RegisterTelegramNativeCommandsParams = Parameters[0]; type GetPluginCommandSpecsFn = typeof import("../plugins/commands.js").getPluginCommandSpecs; type MatchPluginCommandFn = typeof import("../plugins/commands.js").matchPluginCommand; type ExecutePluginCommandFn = typeof import("../plugins/commands.js").executePluginCommand; +type AnyMock = MockFn<(...args: unknown[]) => unknown>; +type AnyAsyncMock = MockFn<(...args: unknown[]) => Promise>; +type NativeCommandHarness = { + handlers: Record Promise>; + sendMessage: AnyAsyncMock; + setMyCommands: AnyAsyncMock; + log: AnyMock; + bot: { + api: { + setMyCommands: AnyAsyncMock; + sendMessage: AnyAsyncMock; + }; + command: (name: string, handler: (ctx: unknown) => Promise) => void; + }; +}; const pluginCommandMocks = vi.hoisted(() => ({ getPluginCommandSpecs: vi.fn(() => []), @@ -86,12 +102,12 @@ export function createNativeCommandsHarness(params?: { nativeEnabled?: boolean; groupConfig?: Record; resolveGroupPolicy?: () => ChannelGroupPolicy; -}) { +}): NativeCommandHarness { const handlers: Record Promise> = {}; - const sendMessage = vi.fn().mockResolvedValue(undefined); - const setMyCommands = vi.fn().mockResolvedValue(undefined); - const log = vi.fn(); - const bot = { + const sendMessage: AnyAsyncMock = vi.fn(async () => undefined); + const setMyCommands: AnyAsyncMock = vi.fn(async () => undefined); + const log: AnyMock = vi.fn(); + const bot: NativeCommandHarness["bot"] = { api: { setMyCommands, sendMessage, @@ -153,7 +169,7 @@ export function createTelegramGroupCommandContext(params?: { }; } -export function findNotAuthorizedCalls(sendMessage: ReturnType) { +export function findNotAuthorizedCalls(sendMessage: AnyAsyncMock) { return sendMessage.mock.calls.filter( (call) => typeof call[1] === "string" && call[1].includes("not authorized"), ); diff --git a/ui/src/i18n/locales/de.ts b/ui/src/i18n/locales/de.ts index f45ffc3f4c0..7fd638766e7 100644 --- a/ui/src/i18n/locales/de.ts +++ b/ui/src/i18n/locales/de.ts @@ -5,6 +5,7 @@ export const de: TranslationMap = { version: "Version", health: "Status", ok: "OK", + online: "Online", offline: "Offline", connect: "Verbinden", refresh: "Aktualisieren", diff --git a/ui/src/i18n/locales/en.ts b/ui/src/i18n/locales/en.ts index df80f2d7c78..370fec9c660 100644 --- a/ui/src/i18n/locales/en.ts +++ b/ui/src/i18n/locales/en.ts @@ -4,6 +4,7 @@ export const en: TranslationMap = { common: { health: "Health", ok: "OK", + online: "Online", offline: "Offline", connect: "Connect", refresh: "Refresh", diff --git a/ui/src/i18n/locales/es.ts b/ui/src/i18n/locales/es.ts index a96ee7ad2d7..091cd2ca937 100644 --- a/ui/src/i18n/locales/es.ts +++ b/ui/src/i18n/locales/es.ts @@ -5,6 +5,7 @@ export const es: TranslationMap = { version: "Versión", health: "Estado", ok: "Correcto", + online: "En línea", offline: "Desconectado", connect: "Conectar", refresh: "Actualizar", diff --git a/ui/src/i18n/locales/pt-BR.ts b/ui/src/i18n/locales/pt-BR.ts index aaaa26c253e..cb9ba1ba283 100644 --- a/ui/src/i18n/locales/pt-BR.ts +++ b/ui/src/i18n/locales/pt-BR.ts @@ -4,6 +4,7 @@ export const pt_BR: TranslationMap = { common: { health: "Saúde", ok: "OK", + online: "Online", offline: "Offline", connect: "Conectar", refresh: "Atualizar", diff --git a/ui/src/i18n/locales/zh-CN.ts b/ui/src/i18n/locales/zh-CN.ts index ac321857253..b039be16f41 100644 --- a/ui/src/i18n/locales/zh-CN.ts +++ b/ui/src/i18n/locales/zh-CN.ts @@ -4,6 +4,7 @@ export const zh_CN: TranslationMap = { common: { health: "健康状况", ok: "正常", + online: "在线", offline: "离线", connect: "连接", refresh: "刷新", diff --git a/ui/src/i18n/locales/zh-TW.ts b/ui/src/i18n/locales/zh-TW.ts index 56a80c61d92..a6a616209e7 100644 --- a/ui/src/i18n/locales/zh-TW.ts +++ b/ui/src/i18n/locales/zh-TW.ts @@ -4,6 +4,7 @@ export const zh_TW: TranslationMap = { common: { health: "健康狀況", ok: "正常", + online: "在線", offline: "離線", connect: "連接", refresh: "刷新", diff --git a/ui/src/styles/chat/layout.css b/ui/src/styles/chat/layout.css index 536acddd29e..f0e0154329d 100644 --- a/ui/src/styles/chat/layout.css +++ b/ui/src/styles/chat/layout.css @@ -9,7 +9,9 @@ flex-direction: column; flex: 1 1 0; height: 100%; - min-height: 0; /* Allow flex shrinking */ + width: 100%; + min-height: 0; + /* Allow flex shrinking */ overflow: hidden; background: transparent !important; border: none !important; @@ -24,8 +26,8 @@ gap: 12px; flex-wrap: nowrap; flex-shrink: 0; - padding-bottom: 12px; - margin-bottom: 12px; + padding-bottom: 0; + margin-bottom: 0; background: transparent; } @@ -49,16 +51,22 @@ /* Chat thread - scrollable middle section, transparent */ .chat-thread { - flex: 1 1 0; /* Grow, shrink, and use 0 base for proper scrolling */ + flex: 1 1 0; + /* Grow, shrink, and use 0 base for proper scrolling */ overflow-y: auto; overflow-x: hidden; - padding: 12px 4px; - margin: 0 -4px; - min-height: 0; /* Allow shrinking for flex scroll behavior */ + padding: 0 6px 6px; + margin: 0 0 0 0; + min-height: 0; + /* Allow shrinking for flex scroll behavior */ border-radius: 12px; background: transparent; } +.chat-thread-inner > :first-child { + margin-top: 0 !important; +} + /* Focus mode exit button */ .chat-focus-exit { position: absolute; @@ -146,7 +154,8 @@ display: flex; flex-direction: column; gap: 12px; - margin-top: auto; /* Push to bottom of flex container */ + margin-top: auto; + /* Push to bottom of flex container */ padding: 12px 4px 4px; background: linear-gradient(to bottom, transparent, var(--bg) 20%); z-index: 10; @@ -163,7 +172,8 @@ border: 1px solid var(--border); width: fit-content; max-width: 100%; - align-self: flex-start; /* Don't stretch in flex column parent */ + align-self: flex-start; + /* Don't stretch in flex column parent */ } .chat-attachment { diff --git a/ui/src/styles/chat/sidebar.css b/ui/src/styles/chat/sidebar.css index 934e285d95b..de6010f3ed7 100644 --- a/ui/src/styles/chat/sidebar.css +++ b/ui/src/styles/chat/sidebar.css @@ -82,6 +82,18 @@ line-height: 1.5; } +.sidebar-markdown .markdown-inline-image { + display: block; + max-width: 100%; + max-height: 420px; + width: auto; + height: auto; + border: 1px solid var(--border); + border-radius: var(--radius-md); + background: color-mix(in srgb, var(--secondary) 70%, transparent); + object-fit: contain; +} + .sidebar-markdown pre { background: rgba(0, 0, 0, 0.12); border-radius: 4px; diff --git a/ui/src/styles/chat/text.css b/ui/src/styles/chat/text.css index 56224fabf9e..dd76434e041 100644 --- a/ui/src/styles/chat/text.css +++ b/ui/src/styles/chat/text.css @@ -56,6 +56,19 @@ font-size: 0.9em; } +.chat-text :where(.markdown-inline-image) { + display: block; + max-width: min(100%, 420px); + max-height: 320px; + width: auto; + height: auto; + margin-top: 0.75em; + border: 1px solid var(--border); + border-radius: var(--radius-md); + background: color-mix(in srgb, var(--secondary) 70%, transparent); + object-fit: contain; +} + .chat-text :where(:not(pre) > code) { background: rgba(0, 0, 0, 0.15); padding: 0.15em 0.4em; diff --git a/ui/src/styles/components.css b/ui/src/styles/components.css index d1dc29ca04e..b2806f3208f 100644 --- a/ui/src/styles/components.css +++ b/ui/src/styles/components.css @@ -2157,7 +2157,7 @@ } .chat-thread { - margin-top: 16px; + margin-top: 0; display: flex; flex-direction: column; gap: 12px; @@ -2165,7 +2165,7 @@ min-height: 0; overflow-y: auto; overflow-x: hidden; - padding: 16px 12px; + padding: 0 12px 16px; min-width: 0; border-radius: 0; border: none; diff --git a/ui/src/styles/layout.css b/ui/src/styles/layout.css index 12f22aef21d..6e19806bb32 100644 --- a/ui/src/styles/layout.css +++ b/ui/src/styles/layout.css @@ -5,7 +5,7 @@ .shell { --shell-pad: 16px; --shell-gap: 16px; - --shell-nav-width: 288px; + --shell-nav-width: 258px; --shell-nav-rail-width: 78px; --shell-topbar-height: 52px; --shell-focus-duration: 200ms; @@ -340,7 +340,7 @@ flex-direction: column; min-height: 0; flex: 1; - padding: 14px 14px 12px; + padding: 14px 10px 12px; border: none; border-radius: 0; background: transparent; @@ -503,7 +503,7 @@ justify-content: space-between; gap: 8px; width: 100%; - padding: 0 12px; + padding: 0 10px; min-height: 28px; background: transparent; border: none; @@ -522,9 +522,9 @@ } .nav-section__label-text { - font-size: 11px; + font-size: 12px; font-weight: 700; - letter-spacing: 0.08em; + letter-spacing: 0.06em; text-transform: uppercase; } @@ -555,9 +555,9 @@ display: flex; align-items: center; justify-content: flex-start; - gap: 10px; - min-height: 38px; - padding: 0 12px; + gap: 8px; + min-height: 40px; + padding: 0 9px; border-radius: 12px; border: 1px solid transparent; background: transparent; @@ -595,8 +595,8 @@ } .nav-item__text { - font-size: 13px; - font-weight: 550; + font-size: 14px; + font-weight: 600; white-space: nowrap; } @@ -763,6 +763,24 @@ margin: 0 auto; } +.sidebar-version__status { + width: 8px; + height: 8px; + border-radius: var(--radius-full); + flex-shrink: 0; + margin-left: auto; +} + +.sidebar-version__status.sidebar-connection-status--online { + background: var(--ok); + box-shadow: 0 0 0 4px color-mix(in srgb, var(--ok) 14%, transparent); +} + +.sidebar-version__status.sidebar-connection-status--offline { + background: var(--danger); + box-shadow: 0 0 0 4px color-mix(in srgb, var(--danger) 14%, transparent); +} + .sidebar--collapsed .sidebar-shell__footer { padding: 8px 0 2px; } @@ -780,6 +798,10 @@ border-radius: 16px; } +.sidebar--collapsed .sidebar-version__status { + margin-left: 0; +} + .shell--nav-collapsed .shell-nav { width: var(--shell-nav-rail-width); min-width: var(--shell-nav-rail-width); @@ -844,7 +866,7 @@ .content--chat { display: flex; flex-direction: column; - gap: 24px; + gap: 2px; overflow: hidden; padding-bottom: 0; } @@ -905,6 +927,7 @@ align-items: center; justify-content: space-between; gap: 16px; + padding-bottom: 0; } .content--chat .content-header > div:first-child { diff --git a/ui/src/styles/layout.mobile.css b/ui/src/styles/layout.mobile.css index 3c929435a7b..036e6a7c588 100644 --- a/ui/src/styles/layout.mobile.css +++ b/ui/src/styles/layout.mobile.css @@ -323,6 +323,10 @@ gap: 8px; } + .content--chat { + gap: 2px; + } + .content--chat .content-header > div:first-child, .content--chat .page-meta, .content--chat .chat-controls { @@ -417,8 +421,8 @@ } .chat-thread { - margin-top: 8px; - padding: 12px 8px; + margin-top: 0; + padding: 0 8px 12px; } .chat-msg { diff --git a/ui/src/ui/app-render.helpers.ts b/ui/src/ui/app-render.helpers.ts index 0ebafc22d4d..eaf94616032 100644 --- a/ui/src/ui/app-render.helpers.ts +++ b/ui/src/ui/app-render.helpers.ts @@ -743,6 +743,23 @@ export function renderTopbarThemeModeToggle(state: AppViewState) { `; } +export function renderSidebarConnectionStatus(state: AppViewState) { + const label = state.connected ? t("common.online") : t("common.offline"); + const toneClass = state.connected + ? "sidebar-connection-status--online" + : "sidebar-connection-status--offline"; + + return html` + + `; +} + export function renderThemeToggle(state: AppViewState) { const setOpen = (orb: HTMLElement, nextOpen: boolean) => { orb.classList.toggle("theme-orb--open", nextOpen); diff --git a/ui/src/ui/app-render.ts b/ui/src/ui/app-render.ts index b1ddf9e323c..643edfca521 100644 --- a/ui/src/ui/app-render.ts +++ b/ui/src/ui/app-render.ts @@ -10,6 +10,7 @@ import { renderChatControls, renderChatSessionSelect, renderTab, + renderSidebarConnectionStatus, renderTopbarThemeModeToggle, } from "./app-render.helpers.ts"; import type { AppViewState } from "./app-view-state.ts"; @@ -437,9 +438,7 @@ export function renderApp(state: AppViewState) { ${t("common.search")} ⌘K -
- ${renderTopbarThemeModeToggle(state)} -
+
${renderTopbarThemeModeToggle(state)}
@@ -543,9 +542,10 @@ export function renderApp(state: AppViewState) { ? html` ${t("common.version")} v${version} + ${renderSidebarConnectionStatus(state)} ` : html` - + ${renderSidebarConnectionStatus(state)} ` } @@ -924,9 +924,21 @@ export function renderApp(state: AppViewState) { state.agentsList?.defaultId ?? state.agentsList?.agents?.[0]?.id ?? null; + if (state.agentsPanel === "files" && refreshedAgentId) { + void loadAgentFiles(state, refreshedAgentId); + } + if (state.agentsPanel === "skills" && refreshedAgentId) { + void loadAgentSkills(state, refreshedAgentId); + } if (state.agentsPanel === "tools" && refreshedAgentId) { void loadToolsCatalog(state, refreshedAgentId); } + if (state.agentsPanel === "channels") { + void loadChannels(state, false); + } + if (state.agentsPanel === "cron") { + void state.loadCron(); + } }, onSelectAgent: (agentId) => { if (state.agentsSelectedId === agentId) { diff --git a/ui/src/ui/controllers/logs.test.ts b/ui/src/ui/controllers/logs.test.ts new file mode 100644 index 00000000000..5d1a830de7a --- /dev/null +++ b/ui/src/ui/controllers/logs.test.ts @@ -0,0 +1,28 @@ +import { describe, expect, it } from "vitest"; +import { parseLogLine } from "./logs.ts"; + +describe("parseLogLine", () => { + it("prefers the human-readable message field when structured data is stored in slot 1", () => { + const line = JSON.stringify({ + 0: '{"subsystem":"gateway/ws"}', + 1: { + cause: "unauthorized", + authReason: "password_missing", + }, + 2: "closed before connect conn=abc code=4008 reason=connect failed", + _meta: { + date: "2026-03-13T19:07:12.128Z", + logLevelName: "WARN", + }, + time: "2026-03-13T14:07:12.138-05:00", + }); + + expect(parseLogLine(line)).toEqual( + expect.objectContaining({ + level: "warn", + subsystem: "gateway/ws", + message: "closed before connect conn=abc code=4008 reason=connect failed", + }), + ); + }); +}); diff --git a/ui/src/ui/controllers/logs.ts b/ui/src/ui/controllers/logs.ts index d2e919c6210..90c2edcf00a 100644 --- a/ui/src/ui/controllers/logs.ts +++ b/ui/src/ui/controllers/logs.ts @@ -77,6 +77,8 @@ export function parseLogLine(line: string): LogEntry { let message: string | null = null; if (typeof obj["1"] === "string") { message = obj["1"]; + } else if (typeof obj["2"] === "string") { + message = obj["2"]; } else if (!contextObj && typeof obj["0"] === "string") { message = obj["0"]; } else if (typeof obj.message === "string") { diff --git a/ui/src/ui/markdown.test.ts b/ui/src/ui/markdown.test.ts index 279cb2b53fb..90bce3b65f5 100644 --- a/ui/src/ui/markdown.test.ts +++ b/ui/src/ui/markdown.test.ts @@ -39,6 +39,7 @@ describe("toSanitizedMarkdownHtml", () => { it("preserves base64 data URI images (#15437)", () => { const html = toSanitizedMarkdownHtml("![Chart](data:image/png;base64,iVBORw0KGgo=)"); expect(html).toContain("`; + return `${escapeHtml(label)}`; }; function normalizeMarkdownImageLabel(text?: string | null): string { diff --git a/ui/src/ui/sidebar-status.browser.test.ts b/ui/src/ui/sidebar-status.browser.test.ts new file mode 100644 index 00000000000..315501c36a2 --- /dev/null +++ b/ui/src/ui/sidebar-status.browser.test.ts @@ -0,0 +1,24 @@ +import { describe, expect, it } from "vitest"; +import { mountApp, registerAppMountHooks } from "./test-helpers/app-mount.ts"; + +registerAppMountHooks(); + +describe("sidebar connection status", () => { + it("shows a single online status dot next to the version", async () => { + const app = mountApp("/chat"); + await app.updateComplete; + + app.hello = { + ok: true, + server: { version: "1.2.3" }, + } as never; + app.requestUpdate(); + await app.updateComplete; + + const version = app.querySelector(".sidebar-version"); + const statusDot = app.querySelector(".sidebar-version__status"); + expect(version).not.toBeNull(); + expect(statusDot).not.toBeNull(); + expect(statusDot?.getAttribute("aria-label")).toContain("Online"); + }); +}); diff --git a/ui/src/ui/views/agents.test.ts b/ui/src/ui/views/agents.test.ts new file mode 100644 index 00000000000..f763877937a --- /dev/null +++ b/ui/src/ui/views/agents.test.ts @@ -0,0 +1,174 @@ +import { render } from "lit"; +import { describe, expect, it } from "vitest"; +import { renderAgents, type AgentsProps } from "./agents.ts"; + +function createSkill() { + return { + name: "Repo Skill", + description: "Skill description", + source: "workspace", + filePath: "/tmp/skill", + baseDir: "/tmp", + skillKey: "repo-skill", + always: false, + disabled: false, + blockedByAllowlist: false, + eligible: true, + requirements: { + bins: [], + env: [], + config: [], + os: [], + }, + missing: { + bins: [], + env: [], + config: [], + os: [], + }, + configChecks: [], + install: [], + }; +} + +function createProps(overrides: Partial = {}): AgentsProps { + return { + basePath: "", + loading: false, + error: null, + agentsList: { + defaultId: "alpha", + mainKey: "main", + scope: "workspace", + agents: [{ id: "alpha", name: "Alpha" } as never, { id: "beta", name: "Beta" } as never], + }, + selectedAgentId: "beta", + activePanel: "overview", + config: { + form: null, + loading: false, + saving: false, + dirty: false, + }, + channels: { + snapshot: null, + loading: false, + error: null, + lastSuccess: null, + }, + cron: { + status: null, + jobs: [], + loading: false, + error: null, + }, + agentFiles: { + list: null, + loading: false, + error: null, + active: null, + contents: {}, + drafts: {}, + saving: false, + }, + agentIdentityLoading: false, + agentIdentityError: null, + agentIdentityById: {}, + agentSkills: { + report: null, + loading: false, + error: null, + agentId: null, + filter: "", + }, + toolsCatalog: { + loading: false, + error: null, + result: null, + }, + onRefresh: () => undefined, + onSelectAgent: () => undefined, + onSelectPanel: () => undefined, + onLoadFiles: () => undefined, + onSelectFile: () => undefined, + onFileDraftChange: () => undefined, + onFileReset: () => undefined, + onFileSave: () => undefined, + onToolsProfileChange: () => undefined, + onToolsOverridesChange: () => undefined, + onConfigReload: () => undefined, + onConfigSave: () => undefined, + onModelChange: () => undefined, + onModelFallbacksChange: () => undefined, + onChannelsRefresh: () => undefined, + onCronRefresh: () => undefined, + onCronRunNow: () => undefined, + onSkillsFilterChange: () => undefined, + onSkillsRefresh: () => undefined, + onAgentSkillToggle: () => undefined, + onAgentSkillsClear: () => undefined, + onAgentSkillsDisableAll: () => undefined, + onSetDefault: () => undefined, + ...overrides, + }; +} + +describe("renderAgents", () => { + it("shows the skills count only for the selected agent's report", async () => { + const container = document.createElement("div"); + render( + renderAgents( + createProps({ + agentSkills: { + report: { + workspaceDir: "/tmp/workspace", + managedSkillsDir: "/tmp/skills", + skills: [createSkill()], + }, + loading: false, + error: null, + agentId: "alpha", + filter: "", + }, + }), + ), + container, + ); + await Promise.resolve(); + + const skillsTab = Array.from(container.querySelectorAll(".agent-tab")).find( + (button) => button.textContent?.includes("Skills"), + ); + + expect(skillsTab?.textContent?.trim()).toBe("Skills"); + }); + + it("shows the selected agent's skills count when the report matches", async () => { + const container = document.createElement("div"); + render( + renderAgents( + createProps({ + agentSkills: { + report: { + workspaceDir: "/tmp/workspace", + managedSkillsDir: "/tmp/skills", + skills: [createSkill()], + }, + loading: false, + error: null, + agentId: "beta", + filter: "", + }, + }), + ), + container, + ); + await Promise.resolve(); + + const skillsTab = Array.from(container.querySelectorAll(".agent-tab")).find( + (button) => button.textContent?.includes("Skills"), + ); + + expect(skillsTab?.textContent?.trim()).toContain("1"); + }); +}); diff --git a/ui/src/ui/views/agents.ts b/ui/src/ui/views/agents.ts index 63917b0f732..4e8b9a065ba 100644 --- a/ui/src/ui/views/agents.ts +++ b/ui/src/ui/views/agents.ts @@ -113,6 +113,10 @@ export function renderAgents(props: AgentsProps) { const selectedAgent = selectedId ? (agents.find((agent) => agent.id === selectedId) ?? null) : null; + const selectedSkillCount = + selectedId && props.agentSkills.agentId === selectedId + ? (props.agentSkills.report?.skills?.length ?? null) + : null; const channelEntryCount = props.channels.snapshot ? Object.keys(props.channels.snapshot.channelAccounts ?? {}).length @@ -122,7 +126,7 @@ export function renderAgents(props: AgentsProps) { : null; const tabCounts: Record = { files: props.agentFiles.list?.files?.length ?? null, - skills: props.agentSkills.report?.skills?.length ?? null, + skills: selectedSkillCount, channels: channelEntryCount, cron: cronJobCount || null, }; From f5b90951087b2e3ead0066c36dd34e2d702a9475 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:42:19 +0000 Subject: [PATCH 08/41] refactor: share zalo send result handling --- extensions/zalo/src/send.ts | 70 ++++++++++++++++++++++--------------- 1 file changed, 42 insertions(+), 28 deletions(-) diff --git a/extensions/zalo/src/send.ts b/extensions/zalo/src/send.ts index 44f1549067a..c6380a3b891 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; @@ -55,14 +77,21 @@ function resolveValidatedSendContext( return { ok: true, chatId: trimmedChatId, token, fetcher }; } +function toInvalidContextResult( + context: ReturnType, +): ZaloSendResult | null { + return context.ok ? null : { ok: false, error: context.error }; +} + export async function sendMessageZalo( chatId: string, text: string, options: ZaloSendOptions = {}, ): Promise { const context = resolveValidatedSendContext(chatId, options); - if (!context.ok) { - return { ok: false, error: context.error }; + const invalidResult = toInvalidContextResult(context); + if (invalidResult) { + return invalidResult; } if (options.mediaUrl) { @@ -73,24 +102,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( @@ -99,16 +120,17 @@ export async function sendPhotoZalo( options: ZaloSendOptions = {}, ): Promise { const context = resolveValidatedSendContext(chatId, options); - if (!context.ok) { - return { ok: false, error: context.error }; + const invalidResult = toInvalidContextResult(context); + if (invalidResult) { + return invalidResult; } if (!photoUrl?.trim()) { 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 +138,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) }; - } + ), + ); } From 853999fd7f8dfa08b5108cbcad526e06f86c46e7 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:42:25 +0000 Subject: [PATCH 09/41] refactor: dedupe synology chat client webhook payloads --- extensions/synology-chat/src/client.ts | 43 ++++++++++++++------------ 1 file changed, 24 insertions(+), 19 deletions(-) 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; From b9f0effd55ea83d045652741d76f08e27b40b761 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:43:01 +0000 Subject: [PATCH 10/41] test: dedupe synology chat client timer setup --- extensions/synology-chat/src/client.test.ts | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) 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(); From 58baf22230e0ec3b816da4dd5114db1734c38469 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:45:01 +0000 Subject: [PATCH 11/41] refactor: share zalo monitor processing context --- extensions/zalo/src/monitor.ts | 175 ++++++++++++++------------------- 1 file changed, 72 insertions(+), 103 deletions(-) diff --git a/extensions/zalo/src/monitor.ts b/extensions/zalo/src/monitor.ts index bd1351bd147..2c5c420ce60 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 } = 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 { From f0d0ad39c4196932bdf3f55c659f677e9e1c0760 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:46:06 +0000 Subject: [PATCH 12/41] test: dedupe nostr profile http assertions --- .../nostr/src/nostr-profile-http.test.ts | 65 +++++++++++-------- 1 file changed, 37 insertions(+), 28 deletions(-) diff --git a/extensions/nostr/src/nostr-profile-http.test.ts b/extensions/nostr/src/nostr-profile-http.test.ts index 8fb17c443f4..745ba8baed5 100644 --- a/extensions/nostr/src/nostr-profile-http.test.ts +++ b/extensions/nostr/src/nostr-profile-http.test.ts @@ -208,6 +208,29 @@ 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 expectOkResponse(res: ReturnType) { + expect(res._getStatusCode()).toBe(200); + const data = JSON.parse(res._getData()); + expect(data.ok).toBe(true); + return data; + } + + 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(); }); From 168394980fe6c8457106c21721864ae166952c18 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:47:08 +0000 Subject: [PATCH 13/41] refactor: share slack allowlist target mapping --- extensions/slack/src/channel.ts | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) 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: { From 8896a477dfab0b27a8cdad9391d8ec15a8ee870c Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:47:45 +0000 Subject: [PATCH 14/41] test: dedupe bluebubbles local media send cases --- extensions/bluebubbles/src/media-send.test.ts | 142 ++++++++++++------ 1 file changed, 94 insertions(+), 48 deletions(-) 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 () => { From d964c1504065bfcc330e721b8667304662c9eed2 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:48:24 +0000 Subject: [PATCH 15/41] test: dedupe synology webhook request helpers --- .../synology-chat/src/webhook-handler.test.ts | 47 +++++++------------ 1 file changed, 18 insertions(+), 29 deletions(-) diff --git a/extensions/synology-chat/src/webhook-handler.test.ts b/extensions/synology-chat/src/webhook-handler.test.ts index 37ee566e6a6..a0b67d49aad 100644 --- a/extensions/synology-chat/src/webhook-handler.test.ts +++ b/extensions/synology-chat/src/webhook-handler.test.ts @@ -37,21 +37,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 +51,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 +118,12 @@ describe("createWebhookHandler", () => { async function expectForbiddenByPolicy(params: { account: Partial; bodyContains: string; + deliver?: ReturnType; }) { + const deliver = params.deliver ?? vi.fn(); const handler = createWebhookHandler({ account: makeAccount(params.account), - deliver: vi.fn(), + deliver, log, }); @@ -137,6 +133,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 +299,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 () => { From 5b51d92f3e825b94ec02376e02bec28d34508fd1 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:52:03 +0000 Subject: [PATCH 16/41] test: dedupe synology channel account fixtures --- extensions/synology-chat/src/channel.test.ts | 139 +++++++------------ 1 file changed, 47 insertions(+), 92 deletions(-) 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); From b23bfef8ccce8bf378c38fb05fcd9c7584da653e Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:52:13 +0000 Subject: [PATCH 17/41] test: dedupe feishu probe fixtures --- extensions/feishu/src/probe.test.ts | 225 ++++++++++++++-------------- 1 file changed, 112 insertions(+), 113 deletions(-) diff --git a/extensions/feishu/src/probe.test.ts b/extensions/feishu/src/probe.test.ts index b93935cccc6..328c83f658a 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 = 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", }); From f2300f4522e0dd4065319865b0db6c95dda69c52 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:53:41 +0000 Subject: [PATCH 18/41] test: dedupe msteams policy route fixtures --- extensions/msteams/src/policy.test.ts | 58 +++++++++++---------------- 1 file changed, 23 insertions(+), 35 deletions(-) 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); From 0530d1c530a47fd00e7a6148158f574e459a4635 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:53:45 +0000 Subject: [PATCH 19/41] test: dedupe twitch access control assertions --- extensions/twitch/src/access-control.test.ts | 51 +++++++++----------- 1 file changed, 23 insertions(+), 28 deletions(-) diff --git a/extensions/twitch/src/access-control.test.ts b/extensions/twitch/src/access-control.test.ts index 874326c9697..1cd2b6345c8 100644 --- a/extensions/twitch/src/access-control.test.ts +++ b/extensions/twitch/src/access-control.test.ts @@ -49,6 +49,21 @@ 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; + } + describe("when no restrictions are configured", () => { it("allows messages that mention the bot (default requireMention)", () => { const result = runAccessCheck({ @@ -109,21 +124,11 @@ 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"); }); @@ -283,21 +288,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"); }); From 110eeec5b8aaae5ece71d5a1ea2cbdaed6b45500 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:54:23 +0000 Subject: [PATCH 20/41] test: dedupe twitch access control checks --- extensions/twitch/src/access-control.test.ts | 92 +++++++------------- 1 file changed, 31 insertions(+), 61 deletions(-) diff --git a/extensions/twitch/src/access-control.test.ts b/extensions/twitch/src/access-control.test.ts index 1cd2b6345c8..3d522246700 100644 --- a/extensions/twitch/src/access-control.test.ts +++ b/extensions/twitch/src/access-control.test.ts @@ -64,6 +64,26 @@ describe("checkTwitchAccessControl", () => { 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({ @@ -134,42 +154,18 @@ describe("checkTwitchAccessControl", () => { }); 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", () => { @@ -193,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"); }); }); From a9d8518e7c2a8138ffffb52435b49c99e4ac93b6 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:54:30 +0000 Subject: [PATCH 21/41] test: dedupe msteams consent auth fixtures --- .../src/monitor-handler.file-consent.test.ts | 59 ++++++++----------- 1 file changed, 26 insertions(+), 33 deletions(-) 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", }); From 25e900f64adedf7b5c34dd6bae366e756bd96c37 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:55:33 +0000 Subject: [PATCH 22/41] test: tighten shared requirements coverage --- src/shared/requirements.test.ts | 35 +++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) 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); + }); }); From 592d93211f0d5dac9df413c55c3ca4c44338ba40 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:57:16 +0000 Subject: [PATCH 23/41] test: tighten shared manifest metadata coverage --- src/shared/entry-metadata.test.ts | 12 ++++++++++ src/shared/entry-status.test.ts | 29 ++++++++++++++++++++++++ src/shared/frontmatter.test.ts | 37 +++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+) 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( From ccced29b4677e356d5ef32f73540f8e31a0ca190 Mon Sep 17 00:00:00 2001 From: Taras Shynkarenko Date: Fri, 13 Mar 2026 20:55:20 +0100 Subject: [PATCH 24/41] perf(build): deduplicate plugin-sdk chunks to fix ~2x memory regression MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bundle all plugin-sdk entries in a single tsdown build pass instead of 38 separate builds. The separate builds prevented the bundler from sharing common chunks, causing massive duplication (e.g. 20 copies of query-expansion, 14 copies of fetch, 11 copies of logger). Measured impact: - dist/ size: 190MB → 64MB (-66%) - plugin-sdk/ size: 142MB → 16MB (-89%) - JS files: 1,395 → 789 (-43%) - 5MB+ files: 27 → 7 (-74%) - Plugin-SDK heap cost: +1,309MB → +63MB (-95%) - Total heap (all chunks loaded): 1,926MB → 711MB (-63%) --- tsdown.config.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) 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", }), From b7ff8256eff5791bb12c22ef2eec265dc82a4405 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:57:16 +0000 Subject: [PATCH 25/41] test: guard plugin-sdk shared-bundle regression (#45426) (thanks @TarasShyn) --- CHANGELOG.md | 1 + src/plugin-sdk/index.test.ts | 79 ++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e61358e91e..19d9bb2347c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- 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. - 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. - Cron/isolated sessions: route nested cron-triggered embedded runner work onto the nested lane so isolated cron jobs no longer deadlock when compaction or other queued inner work runs. Thanks @vincentkoc. diff --git a/src/plugin-sdk/index.test.ts b/src/plugin-sdk/index.test.ts index 24cb7bb67e4..0605cc11c05 100644 --- a/src/plugin-sdk/index.test.ts +++ b/src/plugin-sdk/index.test.ts @@ -1,6 +1,58 @@ +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; + describe("plugin-sdk exports", () => { it("does not expose runtime modules", () => { const forbidden = [ @@ -104,4 +156,31 @@ 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-")); + + 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"); + } + } finally { + await fs.rm(outDir, { recursive: true, force: true }); + } + }); }); From 6a9e141c7afa747b8a08da4368c9d2152f5e7543 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:58:16 +0000 Subject: [PATCH 26/41] test: tighten shared config eval helper coverage --- src/shared/config-eval.test.ts | 77 +++++++++++++++++++++++++++++++++- 1 file changed, 76 insertions(+), 1 deletion(-) 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", () => { From 2fe4c4f8e5cfd89ba02716c2d44df7cfd6559ba6 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:59:35 +0000 Subject: [PATCH 27/41] test: tighten shared auth store coverage --- src/shared/device-auth-store.test.ts | 68 ++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) 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, From fb4aa7eaba572d499f3d56a7b6b78ce4ded04544 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 22:00:22 +0000 Subject: [PATCH 28/41] fix: tighten shared chat envelope coverage --- src/shared/chat-envelope.test.ts | 2 ++ src/shared/chat-envelope.ts | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) 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/); From cb99a23d84bb650b128806dcde07395b198c354e Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 22:02:18 +0000 Subject: [PATCH 29/41] test: tighten shell env helper coverage --- src/infra/shell-env.test.ts | 72 +++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) 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"); From c04ea0eac5693ea20249ce93ab54ffc8f6584ef3 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 22:03:17 +0000 Subject: [PATCH 30/41] test: tighten tmp dir security coverage --- src/infra/tmp-openclaw-dir.test.ts | 71 ++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) 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/); + }); }); From 56e5b8b9e85a3268d56cbaf7e22448bbba69ed78 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 22:04:54 +0000 Subject: [PATCH 31/41] test: tighten secret file error coverage --- src/infra/secret-file.test.ts | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) 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 () => { From 0826feb94d1d71248ecbd2e550086ff10ac0de13 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 22:06:01 +0000 Subject: [PATCH 32/41] test: tighten path prepend helper coverage --- src/infra/path-prepend.test.ts | 48 +++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) 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", + }); + }); }); From fac754041c716ae55f3b04f77a88c6cbb36fdeeb Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 22:07:14 +0000 Subject: [PATCH 33/41] fix: tighten executable path coverage --- src/infra/executable-path.test.ts | 23 +++++++++++++++++++++++ src/infra/executable-path.ts | 4 +++- 2 files changed, 26 insertions(+), 1 deletion(-) 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; From 65f92fd83915a6d01fb2a7e1ef379630bcc0d1ec Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Fri, 13 Mar 2026 18:09:01 -0400 Subject: [PATCH 34/41] Guard updater service refresh against missing invocation cwd (#45486) * Update: capture a stable cwd for service refresh env * Test: cover service refresh when cwd disappears --- src/cli/update-cli.test.ts | 48 ++++++++++++++++++++++++++++ src/cli/update-cli/update-command.ts | 21 ++++++++++-- 2 files changed, 67 insertions(+), 2 deletions(-) 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) { From a66a0852bb2d5fc2ac19e07a32463ba6bb53f39d Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 22:08:35 +0000 Subject: [PATCH 35/41] test: cover plugin-sdk subpath imports --- src/plugin-sdk/index.test.ts | 55 ++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/src/plugin-sdk/index.test.ts b/src/plugin-sdk/index.test.ts index 0605cc11c05..61d1cccb10c 100644 --- a/src/plugin-sdk/index.test.ts +++ b/src/plugin-sdk/index.test.ts @@ -53,6 +53,21 @@ const pluginSdkEntrypoints = [ "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 = [ @@ -159,6 +174,7 @@ describe("plugin-sdk exports", () => { 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({ @@ -179,8 +195,47 @@ describe("plugin-sdk exports", () => { 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 }); } }); }); From d0337a18b6473b3ca165b886267f07e4cb481bf2 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 22:08:54 +0000 Subject: [PATCH 36/41] fix: clear typecheck backlog --- .../src/monitor.webhook-auth.test.ts | 13 +++--- extensions/feishu/src/probe.test.ts | 2 +- .../matrix/src/matrix/monitor/events.test.ts | 42 +++++++++++++------ .../nostr/src/nostr-profile-http.test.ts | 14 +++---- .../synology-chat/src/webhook-handler.test.ts | 3 +- extensions/zalo/src/monitor.ts | 2 +- extensions/zalo/src/send.ts | 16 ++----- .../onboard-non-interactive.gateway.test.ts | 15 +++++-- src/node-host/invoke-system-run.ts | 1 + src/slack/monitor/events/messages.test.ts | 1 - 10 files changed, 63 insertions(+), 46 deletions(-) diff --git a/extensions/bluebubbles/src/monitor.webhook-auth.test.ts b/extensions/bluebubbles/src/monitor.webhook-auth.test.ts index b72b95dc4cc..f6826ac510b 100644 --- a/extensions/bluebubbles/src/monitor.webhook-auth.test.ts +++ b/extensions/bluebubbles/src/monitor.webhook-auth.test.ts @@ -328,13 +328,14 @@ describe("BlueBubbles webhook monitor", () => { } function createHangingWebhookRequest(url = "/bluebubbles-webhook?password=test-password") { - const req = new EventEmitter() as IncomingMessage & { destroy: ReturnType }; + const req = new EventEmitter() as IncomingMessage; + const destroyMock = vi.fn(); req.method = "POST"; req.url = url; req.headers = {}; - req.destroy = vi.fn(); + req.destroy = destroyMock as unknown as IncomingMessage["destroy"]; setRequestRemoteAddress(req, "127.0.0.1"); - return req; + return { req, destroyMock }; } function registerWebhookTargets( @@ -415,7 +416,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(); @@ -427,7 +428,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(); } @@ -436,7 +437,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 328c83f658a..bfc270a4459 100644 --- a/extensions/feishu/src/probe.test.ts +++ b/extensions/feishu/src/probe.test.ts @@ -40,7 +40,7 @@ function setupSuccessClient() { async function expectDefaultSuccessResult( creds = DEFAULT_CREDS, - expected = DEFAULT_SUCCESS_RESULT, + expected: Awaited> = DEFAULT_SUCCESS_RESULT, ) { const result = await probeFeishu(creds); expect(result).toEqual(expected); diff --git a/extensions/matrix/src/matrix/monitor/events.test.ts b/extensions/matrix/src/matrix/monitor/events.test.ts index 3c08a0230d1..6dac0db59fc 100644 --- a/extensions/matrix/src/matrix/monitor/events.test.ts +++ b/extensions/matrix/src/matrix/monitor/events.test.ts @@ -14,6 +14,17 @@ vi.mock("../send.js", () => ({ describe("registerMatrixMonitorEvents", () => { const roomId = "!room:example.org"; + function makeEvent(overrides: Partial): MatrixRawEvent { + return { + event_id: "$event", + sender: "@alice:example.org", + type: "m.room.message", + origin_server_ts: 0, + content: {}, + ...overrides, + }; + } + beforeEach(() => { sendReadReceiptMatrixMock.mockClear(); }); @@ -67,10 +78,10 @@ describe("registerMatrixMonitorEvents", () => { it("sends read receipt immediately for non-self messages", async () => { const { client, onRoomMessage, roomMessageHandler } = createHarness(); - const event = { + const event = makeEvent({ event_id: "$e1", sender: "@alice:example.org", - } as MatrixRawEvent; + }); roomMessageHandler("!room:example.org", event); @@ -81,22 +92,27 @@ describe("registerMatrixMonitorEvents", () => { }); it("does not send read receipts for self messages", async () => { - await expectForwardedWithoutReadReceipt({ - event_id: "$e2", - sender: "@bot:example.org", - }); + await expectForwardedWithoutReadReceipt( + makeEvent({ + event_id: "$e2", + sender: "@bot:example.org", + }), + ); }); it("skips receipt when message lacks sender or event id", async () => { - await expectForwardedWithoutReadReceipt({ - sender: "@alice:example.org", - }); + await expectForwardedWithoutReadReceipt( + makeEvent({ + sender: "@alice:example.org", + event_id: "", + }), + ); }); it("caches self user id across messages", async () => { const { getUserId, roomMessageHandler } = createHarness(); - const first = { event_id: "$e3", sender: "@alice:example.org" } as MatrixRawEvent; - const second = { event_id: "$e4", sender: "@bob:example.org" } as MatrixRawEvent; + 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); @@ -110,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 = { event_id: "$e5", sender: "@alice:example.org" } as MatrixRawEvent; + const event = makeEvent({ event_id: "$e5", sender: "@alice:example.org" }); roomMessageHandler("!room:example.org", event); @@ -126,7 +142,7 @@ describe("registerMatrixMonitorEvents", () => { const { roomMessageHandler, onRoomMessage, getUserId } = createHarness({ getUserId: vi.fn().mockRejectedValue(new Error("cannot resolve self")), }); - const event = { event_id: "$e6", sender: "@alice:example.org" } as MatrixRawEvent; + const event = makeEvent({ event_id: "$e6", sender: "@alice:example.org" }); roomMessageHandler("!room:example.org", event); diff --git a/extensions/nostr/src/nostr-profile-http.test.ts b/extensions/nostr/src/nostr-profile-http.test.ts index 745ba8baed5..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, @@ -217,13 +224,6 @@ describe("nostr-profile-http", () => { }); } - function expectOkResponse(res: ReturnType) { - expect(res._getStatusCode()).toBe(200); - const data = JSON.parse(res._getData()); - expect(data.ok).toBe(true); - return data; - } - function expectBadRequestResponse(res: ReturnType) { expect(res._getStatusCode()).toBe(400); const data = JSON.parse(res._getData()); diff --git a/extensions/synology-chat/src/webhook-handler.test.ts b/extensions/synology-chat/src/webhook-handler.test.ts index a0b67d49aad..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, @@ -118,7 +119,7 @@ describe("createWebhookHandler", () => { async function expectForbiddenByPolicy(params: { account: Partial; bodyContains: string; - deliver?: ReturnType; + deliver?: WebhookHandlerDeps["deliver"]; }) { const deliver = params.deliver ?? vi.fn(); const handler = createWebhookHandler({ diff --git a/extensions/zalo/src/monitor.ts b/extensions/zalo/src/monitor.ts index 2c5c420ce60..d82c0d96ba4 100644 --- a/extensions/zalo/src/monitor.ts +++ b/extensions/zalo/src/monitor.ts @@ -289,7 +289,7 @@ async function handleTextMessage( } async function handleImageMessage(params: ZaloImageMessageParams): Promise { - const { message, mediaMaxMb } = params; + const { message, mediaMaxMb, account, core, runtime } = params; const { photo, caption } = message; let mediaPath: string | undefined; diff --git a/extensions/zalo/src/send.ts b/extensions/zalo/src/send.ts index c6380a3b891..4f35f242191 100644 --- a/extensions/zalo/src/send.ts +++ b/extensions/zalo/src/send.ts @@ -77,21 +77,14 @@ function resolveValidatedSendContext( return { ok: true, chatId: trimmedChatId, token, fetcher }; } -function toInvalidContextResult( - context: ReturnType, -): ZaloSendResult | null { - return context.ok ? null : { ok: false, error: context.error }; -} - export async function sendMessageZalo( chatId: string, text: string, options: ZaloSendOptions = {}, ): Promise { const context = resolveValidatedSendContext(chatId, options); - const invalidResult = toInvalidContextResult(context); - if (invalidResult) { - return invalidResult; + if (!context.ok) { + return { ok: false, error: context.error }; } if (options.mediaUrl) { @@ -120,9 +113,8 @@ export async function sendPhotoZalo( options: ZaloSendOptions = {}, ): Promise { const context = resolveValidatedSendContext(chatId, options); - const invalidResult = toInvalidContextResult(context); - if (invalidResult) { - return invalidResult; + if (!context.ok) { + return { ok: false, error: context.error }; } if (!photoUrl?.trim()) { diff --git a/src/commands/onboard-non-interactive.gateway.test.ts b/src/commands/onboard-non-interactive.gateway.test.ts index 23684eb5f5a..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,11 +409,17 @@ describe("onboard (non-interactive): gateway and remote auth", () => { })); let capturedError = ""; - const runtimeWithCapture = { + const runtimeWithCapture: RuntimeEnv = { log: () => {}, - error: (message: string) => { - capturedError = message; - throw new Error(message); + error: (...args: unknown[]) => { + const firstArg = args[0]; + capturedError = + typeof firstArg === "string" + ? firstArg + : firstArg instanceof Error + ? firstArg.message + : (JSON.stringify(firstArg) ?? ""); + throw new Error(capturedError); }, exit: (_code: number) => { throw new Error("exit should not be reached after runtime.error"); diff --git a/src/node-host/invoke-system-run.ts b/src/node-host/invoke-system-run.ts index 32bd2d6ff79..c38094dc683 100644 --- a/src/node-host/invoke-system-run.ts +++ b/src/node-host/invoke-system-run.ts @@ -34,6 +34,7 @@ import { } from "./invoke-system-run-plan.js"; import type { ExecEventPayload, + ExecFinishedResult, ExecFinishedEventParams, RunResult, SkillBinsProvider, diff --git a/src/slack/monitor/events/messages.test.ts b/src/slack/monitor/events/messages.test.ts index 25fdb77c025..f22b24a44c7 100644 --- a/src/slack/monitor/events/messages.test.ts +++ b/src/slack/monitor/events/messages.test.ts @@ -17,7 +17,6 @@ vi.mock("../../../pairing/pairing-store.js", () => ({ })); type MessageHandler = (args: { event: Record; body: unknown }) => Promise; -type AppMentionHandler = MessageHandler; type RegisteredEventName = "message" | "app_mention"; type MessageCase = { From f59b2b1db32f566514f321efc6c0a546fd33c749 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Fri, 13 Mar 2026 18:10:55 -0400 Subject: [PATCH 37/41] fix(browser): normalize batch act dispatch for selector and batch support (#45457) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(browser): add batch actions, CSS selector support, and click delayMs Adds three improvements to the browser act tool: 1. CSS selector support: All element-targeting actions (click, type, hover, drag, scrollIntoView, select) now accept an optional 'selector' parameter alongside 'ref'. When selector is provided, Playwright's page.locator() is used directly, skipping the need for a snapshot to obtain refs. This reduces roundtrips for agents that already know the DOM structure. 2. Click delay (delayMs): The click action now accepts an optional 'delayMs' parameter. When set, the element is hovered first, then after the specified delay, clicked. This enables human-like hover-before-click in a single tool call instead of three (hover + wait + click). 3. Batch actions: New 'batch' action kind that accepts an array of actions to execute sequentially in a single tool call. Supports 'stopOnError' (default true) to control whether execution halts on first failure. Results are returned as an array. This eliminates the AI inference roundtrip between each action, dramatically reducing latency and token cost for multi-step flows. Addresses: #44431, #38844 * fix(browser): address security review — batch evaluateEnabled guard, input validation, recursion limit Fixes all 4 issues raised by Greptile review: 1. Security: batch actions now respect evaluateEnabled flag. executeSingleAction and batchViaPlaywright accept evaluateEnabled param. evaluate and wait-with-fn inside batches are rejected when evaluateEnabled=false, matching the direct route guards. 2. Security: batch input validation. Each action in body.actions is validated as a plain object with a known kind string before dispatch. Applies same normalization as direct action handlers. 3. Perf: SELECTOR_ALLOWED_KINDS moved to module scope as a ReadonlySet constant (was re-created on every request). 4. Security: max batch nesting depth of 5. Nested batch actions track depth and throw if MAX_BATCH_DEPTH exceeded, preventing call stack exhaustion from crafted payloads. * fix(browser): normalize batch act dispatch * fix(browser): tighten existing-session act typing * fix(browser): preserve batch type text * fix(browser): complete batch action execution * test(browser): cover batch route normalization * test(browser): cover batch interaction dispatch * fix(browser): bound batch route action inputs * fix(browser): harden batch interaction limits * test(browser): cover batch security guardrails --------- Co-authored-by: Diwakar --- CHANGELOG.md | 1 + src/browser/client-actions-core.ts | 36 +- src/browser/client.test.ts | 3 +- src/browser/pw-ai.ts | 1 + .../pw-tools-core.interactions.batch.test.ts | 85 +++ src/browser/pw-tools-core.interactions.ts | 305 +++++++++-- src/browser/pw-tools-core.shared.ts | 15 + src/browser/routes/agent.act.shared.ts | 1 + src/browser/routes/agent.act.ts | 492 ++++++++++++++++-- ...-contract-form-layout-act-commands.test.ts | 186 ++++++- .../server.control-server.test-harness.ts | 3 + 11 files changed, 1029 insertions(+), 99 deletions(-) create mode 100644 src/browser/pw-tools-core.interactions.batch.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 19d9bb2347c..fcf8d4862d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ 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 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/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..2801ebe8190 --- /dev/null +++ b/src/browser/pw-tools-core.interactions.batch.test.ts @@ -0,0 +1,85 @@ +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", + }); + }); +}); diff --git a/src/browser/pw-tools-core.interactions.ts b/src/browser/pw-tools-core.interactions.ts index 852b11bb6dc..dee8a03316c 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,189 @@ 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 + await batchViaPlaywright({ + cdpUrl, + targetId: effectiveTargetId, + actions: action.actions, + stopOnError: action.stopOnError, + evaluateEnabled, + depth: depth + 1, + }); + 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/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..0c4c9e71967 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"; @@ -104,6 +104,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,7 +436,7 @@ 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); } @@ -132,12 +452,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 +473,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 +490,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 +502,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 +526,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 +539,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 +556,7 @@ export function registerBrowserAgentActRoutes( await fillChromeMcpElement({ profileName, targetId: tab.targetId, - uid: ref, + uid: ref!, value: text, }); if (submit) { @@ -230,11 +575,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 +617,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 +638,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 +649,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 +680,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 +691,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 +705,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 +734,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 +747,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 +787,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 +800,7 @@ export function registerBrowserAgentActRoutes( cdpUrl, targetId: tab.targetId, ref, + selector, values, timeoutMs: timeoutMs ?? undefined, }); @@ -627,6 +1022,41 @@ 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"); + } + 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/server.agent-contract-form-layout-act-commands.test.ts b/src/browser/server.agent-contract-form-layout-act-commands.test.ts index 738bf8b7e2d..912d024916c 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", @@ -157,6 +163,130 @@ describe("browser control server", () => { 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("agent contract: hooks + response + downloads + screenshot", async () => { const base = await startServerAndBase(); @@ -165,13 +295,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 +412,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 +501,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 +517,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..3a54f6662d5 100644 --- a/src/browser/server.control-server.test-harness.ts +++ b/src/browser/server.control-server.test-harness.ts @@ -77,6 +77,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 () => {}), @@ -210,7 +211,9 @@ vi.mock("./cdp.js", () => ({ vi.mock("./pw-ai.js", () => pwMocks); 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" })), })); From 1ef0aa443b2f5c9bc4825659d83a532998d8307b Mon Sep 17 00:00:00 2001 From: Eyal En Gad <36604865+eengad@users.noreply.github.com> Date: Fri, 13 Mar 2026 15:14:53 -0700 Subject: [PATCH 38/41] docs(android): note that app is not publicly released yet (#23051) Co-authored-by: Eyal --- docs/platforms/android.md | 2 ++ 1 file changed, 2 insertions(+) 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). From 7e49e98f79073b11134beac27fdff547ba5a4a02 Mon Sep 17 00:00:00 2001 From: Robin Waslander Date: Fri, 13 Mar 2026 23:12:17 +0100 Subject: [PATCH 39/41] fix(telegram): validate webhook secret before reading request body Refs: GHSA-jq3f-vjww-8rq7 --- CHANGELOG.md | 1 + src/telegram/webhook.test.ts | 93 +++++++++++++++++++++++++++++++++++- src/telegram/webhook.ts | 32 ++++++++++++- 3 files changed, 123 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fcf8d4862d8..48290d3389f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- 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. - 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/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) { From e82ba71911ad971d0be9219e9f8d064c61a57746 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Fri, 13 Mar 2026 18:39:28 -0400 Subject: [PATCH 40/41] fix(browser): follow up batch failure and limit handling (#45506) * fix(browser): propagate nested batch failures * fix(browser): validate top-level batch limits * test(browser): cover nested batch failures * test(browser): cover top-level batch limits --- .../pw-tools-core.interactions.batch.test.ts | 19 +++++++++++++++++++ src/browser/pw-tools-core.interactions.ts | 6 +++++- src/browser/routes/agent.act.ts | 3 +++ ...-contract-form-layout-act-commands.test.ts | 16 ++++++++++++++++ 4 files changed, 43 insertions(+), 1 deletion(-) diff --git a/src/browser/pw-tools-core.interactions.batch.test.ts b/src/browser/pw-tools-core.interactions.batch.test.ts index 2801ebe8190..fbd2de4cbc6 100644 --- a/src/browser/pw-tools-core.interactions.batch.test.ts +++ b/src/browser/pw-tools-core.interactions.batch.test.ts @@ -82,4 +82,23 @@ describe("batchViaPlaywright", () => { 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 dee8a03316c..da0efa0c145 100644 --- a/src/browser/pw-tools-core.interactions.ts +++ b/src/browser/pw-tools-core.interactions.ts @@ -845,7 +845,7 @@ async function executeSingleAction( break; case "batch": // Nested batches: delegate recursively - await batchViaPlaywright({ + const nestedResult = await batchViaPlaywright({ cdpUrl, targetId: effectiveTargetId, actions: action.actions, @@ -853,6 +853,10 @@ async function executeSingleAction( 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}`); diff --git a/src/browser/routes/agent.act.ts b/src/browser/routes/agent.act.ts index 0c4c9e71967..05557fe1129 100644 --- a/src/browser/routes/agent.act.ts +++ b/src/browser/routes/agent.act.ts @@ -1043,6 +1043,9 @@ export function registerBrowserAgentActRoutes( 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); 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 912d024916c..16ade600bec 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 @@ -287,6 +287,22 @@ describe("browser control server", () => { 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(); From ae1a1fccfeac598a747a9b4a6c9871c93061229c Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 22:38:28 +0000 Subject: [PATCH 41/41] fix: stabilize browser existing-session control --- src/browser/chrome-mcp.test.ts | 24 +++ src/browser/chrome-mcp.ts | 36 +++- src/browser/profiles-service.ts | 19 +- src/browser/resolved-config-refresh.ts | 16 +- src/browser/routes/agent.act.ts | 47 +++-- .../routes/agent.existing-session.test.ts | 198 ++++++++++++++++++ src/browser/routes/agent.snapshot.ts | 14 +- ...server-context.hot-reload-profiles.test.ts | 1 + ...-contract-form-layout-act-commands.test.ts | 1 - .../server.control-server.test-harness.ts | 102 ++++++++- .../browser-cli-manage.timeout-option.test.ts | 44 ++++ src/cli/browser-cli-manage.ts | 20 +- 12 files changed, 462 insertions(+), 60 deletions(-) create mode 100644 src/browser/routes/agent.existing-session.test.ts 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/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/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.ts b/src/browser/routes/agent.act.ts index 05557fe1129..a8d3a09ce83 100644 --- a/src/browser/routes/agent.act.ts +++ b/src/browser/routes/agent.act.ts @@ -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) { @@ -439,6 +448,17 @@ export function registerBrowserAgentActRoutes( 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, @@ -893,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 && @@ -918,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, @@ -952,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) { 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 16ade600bec..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 @@ -156,7 +156,6 @@ describe("browser control server", () => { kind: "evaluate", fn: "() => 1", }); - expect(res.error).toContain("browser.evaluateEnabled=false"); expect(pwMocks.evaluateViaPlaywright).not.toHaveBeenCalled(); }, diff --git a/src/browser/server.control-server.test-harness.ts b/src/browser/server.control-server.test-harness.ts index 3a54f6662d5..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"); @@ -122,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); @@ -148,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, }; }); @@ -210,6 +285,8 @@ 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 () => {}), @@ -254,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 })) {