diff --git a/AGENTS.md b/AGENTS.md index 5112a8241df..32e706997cb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -132,6 +132,7 @@ - Framework: Vitest with V8 coverage thresholds (70% lines/branches/functions/statements). - Naming: match source names with `*.test.ts`; e2e in `*.e2e.test.ts`. - Run `pnpm test` (or `pnpm test:coverage`) before pushing when you touch logic. +- For targeted/local debugging, keep using the wrapper: `pnpm test -- [vitest args...]` (for example `pnpm test -- src/commands/onboard-search.test.ts -t "shows registered plugin providers"`); do not default to raw `pnpm vitest run ...` because it bypasses wrapper config/profile/pool routing. - Do not set test workers above 16; tried already. - If local Vitest runs cause memory pressure (common on non-Mac-Studio hosts), use `OPENCLAW_TEST_PROFILE=low OPENCLAW_TEST_SERIAL_GATEWAY=1 pnpm test` for land/gate runs. - Live tests (real keys): `CLAWDBOT_LIVE_TEST=1 pnpm test:live` (OpenClaw-only) or `LIVE=1 pnpm test:live` (includes provider live tests). Docker: `pnpm test:docker:live-models`, `pnpm test:docker:live-gateway`. Onboarding Docker E2E: `pnpm test:docker:onboard`. diff --git a/CHANGELOG.md b/CHANGELOG.md index adc886ee1aa..bd28d771de0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ Docs: https://docs.openclaw.ai ### Fixes - Dashboard/chat UI: stop reloading full chat history on every live tool result in dashboard v2 so tool-heavy runs no longer trigger UI freeze/re-render storms while the final event still refreshes persisted history. (#45541) Thanks @BunsDev. +- iMessage/remote attachments: reject unsafe remote attachment paths before spawning SCP, so sender-controlled filenames can no longer inject shell metacharacters into remote media staging. Thanks @lintsinghua. - 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. diff --git a/docs/tools/browser.md b/docs/tools/browser.md index 8a7abe93209..15c0b4b0067 100644 --- a/docs/tools/browser.md +++ b/docs/tools/browser.md @@ -398,6 +398,10 @@ Notes: session only. - OpenClaw uses the official Chrome DevTools MCP `--autoConnect` flow here, not the legacy default-profile remote debugging port workflow. +- Existing-session screenshots support page captures and `--ref` element + captures from snapshots, but not CSS `--element` selectors. +- Existing-session `wait --url` supports exact, substring, and glob patterns + like other browser drivers. `wait --load networkidle` is not supported yet. - Some features still require the extension relay or managed browser path, such as PDF export and download interception. - Leave the relay loopback-only by default. If the relay must be reachable from a different network namespace (for example Gateway in WSL2, Chrome on Windows), set `browser.relayBindHost` to an explicit bind address such as `0.0.0.0` while keeping the surrounding network private and authenticated. diff --git a/extensions/bluebubbles/src/attachments.ts b/extensions/bluebubbles/src/attachments.ts index cbd8a74d807..c5392fd2595 100644 --- a/extensions/bluebubbles/src/attachments.ts +++ b/extensions/bluebubbles/src/attachments.ts @@ -2,7 +2,7 @@ import crypto from "node:crypto"; import path from "node:path"; import type { OpenClawConfig } from "openclaw/plugin-sdk/bluebubbles"; import { resolveBlueBubblesServerAccount } from "./account-resolve.js"; -import { postMultipartFormData } from "./multipart.js"; +import { assertMultipartActionOk, postMultipartFormData } from "./multipart.js"; import { getCachedBlueBubblesPrivateApiStatus, isBlueBubblesPrivateApiStatusEnabled, @@ -262,12 +262,7 @@ export async function sendBlueBubblesAttachment(params: { timeoutMs: opts.timeoutMs ?? 60_000, // longer timeout for file uploads }); - if (!res.ok) { - const errorText = await res.text(); - throw new Error( - `BlueBubbles attachment send failed (${res.status}): ${errorText || "unknown"}`, - ); - } + await assertMultipartActionOk(res, "attachment send"); const responseBody = await res.text(); if (!responseBody) { diff --git a/extensions/bluebubbles/src/chat.test.ts b/extensions/bluebubbles/src/chat.test.ts index cc37829bc9d..f8adc9b86fd 100644 --- a/extensions/bluebubbles/src/chat.test.ts +++ b/extensions/bluebubbles/src/chat.test.ts @@ -29,6 +29,11 @@ describe("chat", () => { }); } + function mockTwoOkTextResponses() { + mockOkTextResponse(); + mockOkTextResponse(); + } + async function expectCalledUrlIncludesPassword(params: { password: string; invoke: () => Promise; @@ -198,15 +203,7 @@ describe("chat", () => { }); it("uses POST for start and DELETE for stop", async () => { - mockFetch - .mockResolvedValueOnce({ - ok: true, - text: () => Promise.resolve(""), - }) - .mockResolvedValueOnce({ - ok: true, - text: () => Promise.resolve(""), - }); + mockTwoOkTextResponses(); await sendBlueBubblesTyping("iMessage;-;+15551234567", true, { serverUrl: "http://localhost:1234", @@ -442,15 +439,7 @@ describe("chat", () => { }); it("adds and removes participant using matching endpoint", async () => { - mockFetch - .mockResolvedValueOnce({ - ok: true, - text: () => Promise.resolve(""), - }) - .mockResolvedValueOnce({ - ok: true, - text: () => Promise.resolve(""), - }); + mockTwoOkTextResponses(); await addBlueBubblesParticipant("chat-guid", "+15551234567", { serverUrl: "http://localhost:1234", diff --git a/extensions/bluebubbles/src/chat.ts b/extensions/bluebubbles/src/chat.ts index 1670f276ba7..17340b7f980 100644 --- a/extensions/bluebubbles/src/chat.ts +++ b/extensions/bluebubbles/src/chat.ts @@ -2,7 +2,7 @@ import crypto from "node:crypto"; import path from "node:path"; import type { OpenClawConfig } from "openclaw/plugin-sdk/bluebubbles"; import { resolveBlueBubblesServerAccount } from "./account-resolve.js"; -import { postMultipartFormData } from "./multipart.js"; +import { assertMultipartActionOk, postMultipartFormData } from "./multipart.js"; import { getCachedBlueBubblesPrivateApiStatus } from "./probe.js"; import { blueBubblesFetchWithTimeout, buildBlueBubblesApiUrl } from "./types.js"; @@ -26,14 +26,6 @@ function assertPrivateApiEnabled(accountId: string, feature: string): void { } } -async function assertBlueBubblesActionOk(response: Response, action: string): Promise { - if (response.ok) { - return; - } - const errorText = await response.text().catch(() => ""); - throw new Error(`BlueBubbles ${action} failed (${response.status}): ${errorText || "unknown"}`); -} - function resolvePartIndex(partIndex: number | undefined): number { return typeof partIndex === "number" ? partIndex : 0; } @@ -63,7 +55,7 @@ async function sendBlueBubblesChatEndpointRequest(params: { { method: params.method }, params.opts.timeoutMs, ); - await assertBlueBubblesActionOk(res, params.action); + await assertMultipartActionOk(res, params.action); } async function sendPrivateApiJsonRequest(params: { @@ -89,7 +81,7 @@ async function sendPrivateApiJsonRequest(params: { } const res = await blueBubblesFetchWithTimeout(url, request, params.opts.timeoutMs); - await assertBlueBubblesActionOk(res, params.action); + await assertMultipartActionOk(res, params.action); } export async function markBlueBubblesChatRead( @@ -327,8 +319,5 @@ export async function setGroupIconBlueBubbles( timeoutMs: opts.timeoutMs ?? 60_000, // longer timeout for file uploads }); - if (!res.ok) { - const errorText = await res.text().catch(() => ""); - throw new Error(`BlueBubbles setGroupIcon failed (${res.status}): ${errorText || "unknown"}`); - } + await assertMultipartActionOk(res, "setGroupIcon"); } diff --git a/extensions/bluebubbles/src/monitor-normalize.test.ts b/extensions/bluebubbles/src/monitor-normalize.test.ts index 3e06302593c..62651279237 100644 --- a/extensions/bluebubbles/src/monitor-normalize.test.ts +++ b/extensions/bluebubbles/src/monitor-normalize.test.ts @@ -1,18 +1,24 @@ import { describe, expect, it } from "vitest"; import { normalizeWebhookMessage, normalizeWebhookReaction } from "./monitor-normalize.js"; +function createFallbackDmPayload(overrides: Record = {}) { + return { + guid: "msg-1", + isGroup: false, + isFromMe: false, + handle: null, + chatGuid: "iMessage;-;+15551234567", + ...overrides, + }; +} + describe("normalizeWebhookMessage", () => { it("falls back to DM chatGuid handle when sender handle is missing", () => { const result = normalizeWebhookMessage({ type: "new-message", - data: { - guid: "msg-1", + data: createFallbackDmPayload({ text: "hello", - isGroup: false, - isFromMe: false, - handle: null, - chatGuid: "iMessage;-;+15551234567", - }, + }), }); expect(result).not.toBeNull(); @@ -78,15 +84,11 @@ describe("normalizeWebhookReaction", () => { it("falls back to DM chatGuid handle when reaction sender handle is missing", () => { const result = normalizeWebhookReaction({ type: "updated-message", - data: { + data: createFallbackDmPayload({ guid: "msg-2", associatedMessageGuid: "p:0/msg-1", associatedMessageType: 2000, - isGroup: false, - isFromMe: false, - handle: null, - chatGuid: "iMessage;-;+15551234567", - }, + }), }); expect(result).not.toBeNull(); diff --git a/extensions/bluebubbles/src/multipart.ts b/extensions/bluebubbles/src/multipart.ts index 851cca016b7..e7c840745bb 100644 --- a/extensions/bluebubbles/src/multipart.ts +++ b/extensions/bluebubbles/src/multipart.ts @@ -30,3 +30,11 @@ export async function postMultipartFormData(params: { params.timeoutMs, ); } + +export async function assertMultipartActionOk(response: Response, action: string): Promise { + if (response.ok) { + return; + } + const errorText = await response.text().catch(() => ""); + throw new Error(`BlueBubbles ${action} failed (${response.status}): ${errorText || "unknown"}`); +} diff --git a/extensions/device-pair/index.ts b/extensions/device-pair/index.ts index 825d1668ac0..7ba88842a7a 100644 --- a/extensions/device-pair/index.ts +++ b/extensions/device-pair/index.ts @@ -108,13 +108,21 @@ function resolveScheme( return cfg.gateway?.tls?.enabled === true ? "wss" : "ws"; } -function isPrivateIPv4(address: string): boolean { +function parseIPv4Octets(address: string): [number, number, number, number] | null { const parts = address.split("."); - if (parts.length != 4) { - return false; + if (parts.length !== 4) { + return null; } const octets = parts.map((part) => Number.parseInt(part, 10)); if (octets.some((value) => !Number.isFinite(value) || value < 0 || value > 255)) { + return null; + } + return octets as [number, number, number, number]; +} + +function isPrivateIPv4(address: string): boolean { + const octets = parseIPv4Octets(address); + if (!octets) { return false; } const [a, b] = octets; @@ -131,12 +139,8 @@ function isPrivateIPv4(address: string): boolean { } function isTailnetIPv4(address: string): boolean { - const parts = address.split("."); - if (parts.length !== 4) { - return false; - } - const octets = parts.map((part) => Number.parseInt(part, 10)); - if (octets.some((value) => !Number.isFinite(value) || value < 0 || value > 255)) { + const octets = parseIPv4Octets(address); + if (!octets) { return false; } const [a, b] = octets; diff --git a/extensions/diffs/src/http.test.ts b/extensions/diffs/src/http.test.ts index 43216580379..a1caef018e4 100644 --- a/extensions/diffs/src/http.test.ts +++ b/extensions/diffs/src/http.test.ts @@ -9,6 +9,19 @@ describe("createDiffsHttpHandler", () => { let store: DiffArtifactStore; let cleanupRootDir: () => Promise; + async function handleLocalGet(url: string) { + const handler = createDiffsHttpHandler({ store }); + const res = createMockServerResponse(); + const handled = await handler( + localReq({ + method: "GET", + url, + }), + res, + ); + return { handled, res }; + } + beforeEach(async () => { ({ store, cleanup: cleanupRootDir } = await createDiffStoreHarness("openclaw-diffs-http-")); }); @@ -19,16 +32,7 @@ describe("createDiffsHttpHandler", () => { it("serves a stored diff document", async () => { const artifact = await createViewerArtifact(store); - - const handler = createDiffsHttpHandler({ store }); - const res = createMockServerResponse(); - const handled = await handler( - localReq({ - method: "GET", - url: artifact.viewerPath, - }), - res, - ); + const { handled, res } = await handleLocalGet(artifact.viewerPath); expect(handled).toBe(true); expect(res.statusCode).toBe(200); @@ -38,15 +42,8 @@ describe("createDiffsHttpHandler", () => { it("rejects invalid tokens", async () => { const artifact = await createViewerArtifact(store); - - const handler = createDiffsHttpHandler({ store }); - const res = createMockServerResponse(); - const handled = await handler( - localReq({ - method: "GET", - url: artifact.viewerPath.replace(artifact.token, "bad-token"), - }), - res, + const { handled, res } = await handleLocalGet( + artifact.viewerPath.replace(artifact.token, "bad-token"), ); expect(handled).toBe(true); diff --git a/extensions/diffs/src/tool.test.ts b/extensions/diffs/src/tool.test.ts index 416bdf8dc14..1e939c60390 100644 --- a/extensions/diffs/src/tool.test.ts +++ b/extensions/diffs/src/tool.test.ts @@ -135,9 +135,7 @@ describe("diffs tool", () => { mode: "file", }); - expect(screenshotter.screenshotHtml).toHaveBeenCalledTimes(1); - expect((result?.details as Record).mode).toBe("file"); - expect((result?.details as Record).viewerUrl).toBeUndefined(); + expectArtifactOnlyFileResult(screenshotter, result); }); it("honors ttlSeconds for artifact-only file output", async () => { @@ -227,9 +225,7 @@ describe("diffs tool", () => { after: "two\n", }); - expect(screenshotter.screenshotHtml).toHaveBeenCalledTimes(1); - expect((result?.details as Record).mode).toBe("file"); - expect((result?.details as Record).viewerUrl).toBeUndefined(); + expectArtifactOnlyFileResult(screenshotter, result); }); it("falls back to view output when both mode cannot render an image", async () => { @@ -434,6 +430,15 @@ function createToolWithScreenshotter( }); } +function expectArtifactOnlyFileResult( + screenshotter: DiffScreenshotter, + result: { details?: Record } | null | undefined, +) { + expect(screenshotter.screenshotHtml).toHaveBeenCalledTimes(1); + expect((result?.details as Record).mode).toBe("file"); + expect((result?.details as Record).viewerUrl).toBeUndefined(); +} + function createPngScreenshotter( params: { assertHtml?: (html: string) => void; diff --git a/extensions/discord/src/subagent-hooks.test.ts b/extensions/discord/src/subagent-hooks.test.ts index d58f07c1314..6d5824f69ae 100644 --- a/extensions/discord/src/subagent-hooks.test.ts +++ b/extensions/discord/src/subagent-hooks.test.ts @@ -75,6 +75,27 @@ function getRequiredHandler( return handler; } +function resolveSubagentDeliveryTargetForTest(requesterOrigin: { + channel: string; + accountId: string; + to: string; + threadId?: string; +}) { + const handlers = registerHandlersForTest(); + const handler = getRequiredHandler(handlers, "subagent_delivery_target"); + return handler( + { + childSessionKey: "agent:main:subagent:child", + requesterSessionKey: "agent:main:main", + requesterOrigin, + childRunId: "run-1", + spawnMode: "session", + expectsCompletionMessage: true, + }, + {}, + ); +} + function createSpawnEvent(overrides?: { childSessionKey?: string; agentId?: string; @@ -324,25 +345,12 @@ describe("discord subagent hook handlers", () => { hookMocks.listThreadBindingsBySessionKey.mockReturnValueOnce([ { accountId: "work", threadId: "777" }, ]); - const handlers = registerHandlersForTest(); - const handler = getRequiredHandler(handlers, "subagent_delivery_target"); - - const result = handler( - { - childSessionKey: "agent:main:subagent:child", - requesterSessionKey: "agent:main:main", - requesterOrigin: { - channel: "discord", - accountId: "work", - to: "channel:123", - threadId: "777", - }, - childRunId: "run-1", - spawnMode: "session", - expectsCompletionMessage: true, - }, - {}, - ); + const result = resolveSubagentDeliveryTargetForTest({ + channel: "discord", + accountId: "work", + to: "channel:123", + threadId: "777", + }); expect(hookMocks.listThreadBindingsBySessionKey).toHaveBeenCalledWith({ targetSessionKey: "agent:main:subagent:child", @@ -364,24 +372,11 @@ describe("discord subagent hook handlers", () => { { accountId: "work", threadId: "777" }, { accountId: "work", threadId: "888" }, ]); - const handlers = registerHandlersForTest(); - const handler = getRequiredHandler(handlers, "subagent_delivery_target"); - - const result = handler( - { - childSessionKey: "agent:main:subagent:child", - requesterSessionKey: "agent:main:main", - requesterOrigin: { - channel: "discord", - accountId: "work", - to: "channel:123", - }, - childRunId: "run-1", - spawnMode: "session", - expectsCompletionMessage: true, - }, - {}, - ); + const result = resolveSubagentDeliveryTargetForTest({ + channel: "discord", + accountId: "work", + to: "channel:123", + }); expect(result).toBeUndefined(); }); diff --git a/extensions/feishu/src/accounts.test.ts b/extensions/feishu/src/accounts.test.ts index 56783bbd29d..cfe8d0abcdc 100644 --- a/extensions/feishu/src/accounts.test.ts +++ b/extensions/feishu/src/accounts.test.ts @@ -9,6 +9,23 @@ import type { FeishuConfig } from "./types.js"; const asConfig = (value: Partial) => value as FeishuConfig; +function makeDefaultAndRouterAccounts() { + return { + default: { appId: "cli_default", appSecret: "secret_default" }, // pragma: allowlist secret + "router-d": { appId: "cli_router", appSecret: "secret_router" }, // pragma: allowlist secret + }; +} + +function expectExplicitDefaultAccountSelection( + account: ReturnType, + appId: string, +) { + expect(account.accountId).toBe("router-d"); + expect(account.selectionSource).toBe("explicit-default"); + expect(account.configured).toBe(true); + expect(account.appId).toBe(appId); +} + function withEnvVar(key: string, value: string | undefined, run: () => void) { const prev = process.env[key]; if (value === undefined) { @@ -44,10 +61,7 @@ describe("resolveDefaultFeishuAccountId", () => { channels: { feishu: { defaultAccount: "router-d", - accounts: { - default: { appId: "cli_default", appSecret: "secret_default" }, // pragma: allowlist secret - "router-d": { appId: "cli_router", appSecret: "secret_router" }, // pragma: allowlist secret - }, + accounts: makeDefaultAndRouterAccounts(), }, }, }; @@ -278,10 +292,7 @@ describe("resolveFeishuAccount", () => { }; const account = resolveFeishuAccount({ cfg: cfg as never, accountId: undefined }); - expect(account.accountId).toBe("router-d"); - expect(account.selectionSource).toBe("explicit-default"); - expect(account.configured).toBe(true); - expect(account.appId).toBe("top_level_app"); + expectExplicitDefaultAccountSelection(account, "top_level_app"); }); it("uses configured default account when accountId is omitted", () => { @@ -298,10 +309,7 @@ describe("resolveFeishuAccount", () => { }; const account = resolveFeishuAccount({ cfg: cfg as never, accountId: undefined }); - expect(account.accountId).toBe("router-d"); - expect(account.selectionSource).toBe("explicit-default"); - expect(account.configured).toBe(true); - expect(account.appId).toBe("cli_router"); + expectExplicitDefaultAccountSelection(account, "cli_router"); }); it("keeps explicit accountId selection", () => { @@ -309,10 +317,7 @@ describe("resolveFeishuAccount", () => { channels: { feishu: { defaultAccount: "router-d", - accounts: { - default: { appId: "cli_default", appSecret: "secret_default" }, // pragma: allowlist secret - "router-d": { appId: "cli_router", appSecret: "secret_router" }, // pragma: allowlist secret - }, + accounts: makeDefaultAndRouterAccounts(), }, }, }; diff --git a/extensions/feishu/src/config-schema.test.ts b/extensions/feishu/src/config-schema.test.ts index 0e0881c849f..aacbac85062 100644 --- a/extensions/feishu/src/config-schema.test.ts +++ b/extensions/feishu/src/config-schema.test.ts @@ -1,6 +1,16 @@ import { describe, expect, it } from "vitest"; import { FeishuConfigSchema, FeishuGroupSchema } from "./config-schema.js"; +function expectSchemaIssue( + result: ReturnType, + issuePath: string, +) { + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.issues.some((issue) => issue.path.join(".") === issuePath)).toBe(true); + } +} + describe("FeishuConfigSchema webhook validation", () => { it("applies top-level defaults", () => { const result = FeishuConfigSchema.parse({}); @@ -39,12 +49,7 @@ describe("FeishuConfigSchema webhook validation", () => { appSecret: "secret_top", // pragma: allowlist secret }); - expect(result.success).toBe(false); - if (!result.success) { - expect( - result.error.issues.some((issue) => issue.path.join(".") === "verificationToken"), - ).toBe(true); - } + expectSchemaIssue(result, "verificationToken"); }); it("rejects top-level webhook mode without encryptKey", () => { @@ -55,10 +60,7 @@ describe("FeishuConfigSchema webhook validation", () => { appSecret: "secret_top", // pragma: allowlist secret }); - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error.issues.some((issue) => issue.path.join(".") === "encryptKey")).toBe(true); - } + expectSchemaIssue(result, "encryptKey"); }); it("accepts top-level webhook mode with verificationToken and encryptKey", () => { @@ -84,14 +86,7 @@ describe("FeishuConfigSchema webhook validation", () => { }, }); - expect(result.success).toBe(false); - if (!result.success) { - expect( - result.error.issues.some( - (issue) => issue.path.join(".") === "accounts.main.verificationToken", - ), - ).toBe(true); - } + expectSchemaIssue(result, "accounts.main.verificationToken"); }); it("rejects account webhook mode without encryptKey", () => { @@ -106,12 +101,7 @@ describe("FeishuConfigSchema webhook validation", () => { }, }); - expect(result.success).toBe(false); - if (!result.success) { - expect( - result.error.issues.some((issue) => issue.path.join(".") === "accounts.main.encryptKey"), - ).toBe(true); - } + expectSchemaIssue(result, "accounts.main.encryptKey"); }); it("accepts account webhook mode inheriting top-level verificationToken and encryptKey", () => { diff --git a/extensions/feishu/src/media.test.ts b/extensions/feishu/src/media.test.ts index 813e5090292..b0226669df1 100644 --- a/extensions/feishu/src/media.test.ts +++ b/extensions/feishu/src/media.test.ts @@ -64,18 +64,21 @@ function expectMediaTimeoutClientConfigured(): void { ); } +function mockResolvedFeishuAccount() { + resolveFeishuAccountMock.mockReturnValue({ + configured: true, + accountId: "main", + config: {}, + appId: "app_id", + appSecret: "app_secret", + domain: "feishu", + }); +} + describe("sendMediaFeishu msg_type routing", () => { beforeEach(() => { vi.clearAllMocks(); - - resolveFeishuAccountMock.mockReturnValue({ - configured: true, - accountId: "main", - config: {}, - appId: "app_id", - appSecret: "app_secret", - domain: "feishu", - }); + mockResolvedFeishuAccount(); normalizeFeishuTargetMock.mockReturnValue("ou_target"); resolveReceiveIdTypeMock.mockReturnValue("open_id"); @@ -483,15 +486,7 @@ describe("sanitizeFileNameForUpload", () => { describe("downloadMessageResourceFeishu", () => { beforeEach(() => { vi.clearAllMocks(); - - resolveFeishuAccountMock.mockReturnValue({ - configured: true, - accountId: "main", - config: {}, - appId: "app_id", - appSecret: "app_secret", - domain: "feishu", - }); + mockResolvedFeishuAccount(); createFeishuClientMock.mockReturnValue({ im: { diff --git a/extensions/feishu/src/monitor.reaction.test.ts b/extensions/feishu/src/monitor.reaction.test.ts index e17859d0531..6d3f64a32d0 100644 --- a/extensions/feishu/src/monitor.reaction.test.ts +++ b/extensions/feishu/src/monitor.reaction.test.ts @@ -78,6 +78,25 @@ async function resolveReactionWithLookup(params: { }); } +async function resolveNonBotReaction(params?: { cfg?: ClawdbotConfig; uuid?: () => string }) { + return await resolveReactionSyntheticEvent({ + cfg: params?.cfg ?? cfg, + accountId: "default", + event: makeReactionEvent(), + botOpenId: "ou_bot", + fetchMessage: async () => ({ + messageId: "om_msg1", + chatId: "oc_group", + chatType: "group", + senderOpenId: "ou_other", + senderType: "user", + content: "hello", + contentType: "text", + }), + ...(params?.uuid ? { uuid: params.uuid } : {}), + }); +} + type FeishuMention = NonNullable[number]; function buildDebounceConfig(): ClawdbotConfig { @@ -179,6 +198,19 @@ function getFirstDispatchedEvent(): FeishuMessageEvent { return firstParams.event; } +function expectSingleDispatchedEvent(): FeishuMessageEvent { + expect(handleFeishuMessageMock).toHaveBeenCalledTimes(1); + return getFirstDispatchedEvent(); +} + +function expectParsedFirstDispatchedEvent(botOpenId = "ou_bot") { + const dispatched = expectSingleDispatchedEvent(); + return { + dispatched, + parsed: parseFeishuMessageEvent(dispatched, botOpenId), + }; +} + function setDedupPassThroughMocks(): void { vi.spyOn(dedup, "tryRecordMessage").mockReturnValue(true); vi.spyOn(dedup, "tryRecordMessagePersistent").mockResolvedValue(true); @@ -203,6 +235,13 @@ async function enqueueDebouncedMessage( await Promise.resolve(); } +function setStaleRetryMocks(messageId = "om_old") { + vi.spyOn(dedup, "hasRecordedMessage").mockImplementation((key) => key.endsWith(`:${messageId}`)); + vi.spyOn(dedup, "hasRecordedMessagePersistent").mockImplementation( + async (currentMessageId) => currentMessageId === messageId, + ); +} + describe("resolveReactionSyntheticEvent", () => { it("filters app self-reactions", async () => { const event = makeReactionEvent({ operator_type: "app" }); @@ -262,28 +301,12 @@ describe("resolveReactionSyntheticEvent", () => { }); it("filters reactions on non-bot messages", async () => { - const event = makeReactionEvent(); - const result = await resolveReactionSyntheticEvent({ - cfg, - accountId: "default", - event, - botOpenId: "ou_bot", - fetchMessage: async () => ({ - messageId: "om_msg1", - chatId: "oc_group", - chatType: "group", - senderOpenId: "ou_other", - senderType: "user", - content: "hello", - contentType: "text", - }), - }); + const result = await resolveNonBotReaction(); expect(result).toBeNull(); }); it("allows non-bot reactions when reactionNotifications is all", async () => { - const event = makeReactionEvent(); - const result = await resolveReactionSyntheticEvent({ + const result = await resolveNonBotReaction({ cfg: { channels: { feishu: { @@ -291,18 +314,6 @@ describe("resolveReactionSyntheticEvent", () => { }, }, } as ClawdbotConfig, - accountId: "default", - event, - botOpenId: "ou_bot", - fetchMessage: async () => ({ - messageId: "om_msg1", - chatId: "oc_group", - chatType: "group", - senderOpenId: "ou_other", - senderType: "user", - content: "hello", - contentType: "text", - }), uuid: () => "fixed-uuid", }); expect(result?.message.message_id).toBe("om_msg1:reaction:THUMBSUP:fixed-uuid"); @@ -457,8 +468,7 @@ describe("Feishu inbound debounce regressions", () => { ); await vi.advanceTimersByTimeAsync(25); - expect(handleFeishuMessageMock).toHaveBeenCalledTimes(1); - const dispatched = getFirstDispatchedEvent(); + const dispatched = expectSingleDispatchedEvent(); const mergedMentions = dispatched.message.mentions ?? []; expect(mergedMentions.some((mention) => mention.id.open_id === "ou_bot")).toBe(true); expect(mergedMentions.some((mention) => mention.id.open_id === "ou_user_a")).toBe(false); @@ -517,9 +527,7 @@ describe("Feishu inbound debounce regressions", () => { ); await vi.advanceTimersByTimeAsync(25); - expect(handleFeishuMessageMock).toHaveBeenCalledTimes(1); - const dispatched = getFirstDispatchedEvent(); - const parsed = parseFeishuMessageEvent(dispatched, "ou_bot"); + const { dispatched, parsed } = expectParsedFirstDispatchedEvent(); expect(parsed.mentionedBot).toBe(true); expect(parsed.mentionTargets).toBeUndefined(); const mergedMentions = dispatched.message.mentions ?? []; @@ -547,19 +555,14 @@ describe("Feishu inbound debounce regressions", () => { ); await vi.advanceTimersByTimeAsync(25); - expect(handleFeishuMessageMock).toHaveBeenCalledTimes(1); - const dispatched = getFirstDispatchedEvent(); - const parsed = parseFeishuMessageEvent(dispatched, "ou_bot"); + const { parsed } = expectParsedFirstDispatchedEvent(); expect(parsed.mentionedBot).toBe(true); }); it("excludes previously processed retries from combined debounce text", async () => { vi.spyOn(dedup, "tryRecordMessage").mockReturnValue(true); vi.spyOn(dedup, "tryRecordMessagePersistent").mockResolvedValue(true); - vi.spyOn(dedup, "hasRecordedMessage").mockImplementation((key) => key.endsWith(":om_old")); - vi.spyOn(dedup, "hasRecordedMessagePersistent").mockImplementation( - async (messageId) => messageId === "om_old", - ); + setStaleRetryMocks(); const onMessage = await setupDebounceMonitor(); await onMessage(createTextEvent({ messageId: "om_old", text: "stale" })); @@ -576,8 +579,7 @@ describe("Feishu inbound debounce regressions", () => { await Promise.resolve(); await vi.advanceTimersByTimeAsync(25); - expect(handleFeishuMessageMock).toHaveBeenCalledTimes(1); - const dispatched = getFirstDispatchedEvent(); + const dispatched = expectSingleDispatchedEvent(); expect(dispatched.message.message_id).toBe("om_new_2"); const combined = JSON.parse(dispatched.message.content) as { text?: string }; expect(combined.text).toBe("first\nsecond"); @@ -586,10 +588,7 @@ describe("Feishu inbound debounce regressions", () => { it("uses latest fresh message id when debounce batch ends with stale retry", async () => { const recordSpy = vi.spyOn(dedup, "tryRecordMessage").mockReturnValue(true); vi.spyOn(dedup, "tryRecordMessagePersistent").mockResolvedValue(true); - vi.spyOn(dedup, "hasRecordedMessage").mockImplementation((key) => key.endsWith(":om_old")); - vi.spyOn(dedup, "hasRecordedMessagePersistent").mockImplementation( - async (messageId) => messageId === "om_old", - ); + setStaleRetryMocks(); const onMessage = await setupDebounceMonitor(); await onMessage(createTextEvent({ messageId: "om_new", text: "fresh" })); @@ -600,8 +599,7 @@ describe("Feishu inbound debounce regressions", () => { await Promise.resolve(); await vi.advanceTimersByTimeAsync(25); - expect(handleFeishuMessageMock).toHaveBeenCalledTimes(1); - const dispatched = getFirstDispatchedEvent(); + const dispatched = expectSingleDispatchedEvent(); expect(dispatched.message.message_id).toBe("om_new"); const combined = JSON.parse(dispatched.message.content) as { text?: string }; expect(combined.text).toBe("fresh"); diff --git a/extensions/feishu/src/monitor.startup.test.ts b/extensions/feishu/src/monitor.startup.test.ts index f5e19159f0a..54ad94620ea 100644 --- a/extensions/feishu/src/monitor.startup.test.ts +++ b/extensions/feishu/src/monitor.startup.test.ts @@ -1,35 +1,19 @@ import type { ClawdbotConfig } from "openclaw/plugin-sdk/feishu"; import { afterEach, describe, expect, it, vi } from "vitest"; import { monitorFeishuProvider, stopFeishuMonitor } from "./monitor.js"; +import { + createFeishuClientMockModule, + createFeishuRuntimeMockModule, +} from "./monitor.test-mocks.js"; const probeFeishuMock = vi.hoisted(() => vi.fn()); -const feishuClientMockModule = vi.hoisted(() => ({ - createFeishuWSClient: vi.fn(() => ({ start: vi.fn() })), - createEventDispatcher: vi.fn(() => ({ register: vi.fn() })), -})); -const feishuRuntimeMockModule = vi.hoisted(() => ({ - getFeishuRuntime: () => ({ - channel: { - debounce: { - resolveInboundDebounceMs: () => 0, - createInboundDebouncer: () => ({ - enqueue: async () => {}, - flushKey: async () => {}, - }), - }, - text: { - hasControlCommand: () => false, - }, - }, - }), -})); vi.mock("./probe.js", () => ({ probeFeishu: probeFeishuMock, })); -vi.mock("./client.js", () => feishuClientMockModule); -vi.mock("./runtime.js", () => feishuRuntimeMockModule); +vi.mock("./client.js", () => createFeishuClientMockModule()); +vi.mock("./runtime.js", () => createFeishuRuntimeMockModule()); function buildMultiAccountWebsocketConfig(accountIds: string[]): ClawdbotConfig { return { @@ -52,6 +36,12 @@ function buildMultiAccountWebsocketConfig(accountIds: string[]): ClawdbotConfig } as ClawdbotConfig; } +async function waitForStartedAccount(started: string[], accountId: string) { + for (let i = 0; i < 10 && !started.includes(accountId); i += 1) { + await Promise.resolve(); + } +} + afterEach(() => { stopFeishuMonitor(); }); @@ -116,10 +106,7 @@ describe("Feishu monitor startup preflight", () => { }); try { - for (let i = 0; i < 10 && !started.includes("beta"); i += 1) { - await Promise.resolve(); - } - + await waitForStartedAccount(started, "beta"); expect(started).toEqual(["alpha", "beta"]); expect(started.filter((accountId) => accountId === "alpha")).toHaveLength(1); } finally { @@ -153,10 +140,7 @@ describe("Feishu monitor startup preflight", () => { }); try { - for (let i = 0; i < 10 && !started.includes("beta"); i += 1) { - await Promise.resolve(); - } - + await waitForStartedAccount(started, "beta"); expect(started).toEqual(["alpha", "beta"]); expect(runtime.error).toHaveBeenCalledWith( expect.stringContaining("bot info probe timed out"), diff --git a/extensions/feishu/src/monitor.webhook-e2e.test.ts b/extensions/feishu/src/monitor.webhook-e2e.test.ts index 451ebe0d2bf..a11957e3393 100644 --- a/extensions/feishu/src/monitor.webhook-e2e.test.ts +++ b/extensions/feishu/src/monitor.webhook-e2e.test.ts @@ -50,6 +50,14 @@ function encryptFeishuPayload(encryptKey: string, payload: Record) { + return await fetch(url, { + method: "POST", + headers: signFeishuPayload({ encryptKey: "encrypt_key", payload }), + body: JSON.stringify(payload), + }); +} + afterEach(() => { stopFeishuMonitor(); }); @@ -143,11 +151,7 @@ describe("Feishu webhook signed-request e2e", () => { monitorFeishuProvider, async (url) => { const payload = { type: "url_verification", challenge: "challenge-token" }; - const response = await fetch(url, { - method: "POST", - headers: signFeishuPayload({ encryptKey: "encrypt_key", payload }), - body: JSON.stringify(payload), - }); + const response = await postSignedPayload(url, payload); expect(response.status).toBe(200); await expect(response.json()).resolves.toEqual({ challenge: "challenge-token" }); @@ -172,11 +176,7 @@ describe("Feishu webhook signed-request e2e", () => { header: { event_type: "unknown.event" }, event: {}, }; - const response = await fetch(url, { - method: "POST", - headers: signFeishuPayload({ encryptKey: "encrypt_key", payload }), - body: JSON.stringify(payload), - }); + const response = await postSignedPayload(url, payload); expect(response.status).toBe(200); expect(await response.text()).toContain("no unknown.event event handle"); @@ -202,11 +202,7 @@ describe("Feishu webhook signed-request e2e", () => { challenge: "encrypted-challenge-token", }), }; - const response = await fetch(url, { - method: "POST", - headers: signFeishuPayload({ encryptKey: "encrypt_key", payload }), - body: JSON.stringify(payload), - }); + const response = await postSignedPayload(url, payload); expect(response.status).toBe(200); await expect(response.json()).resolves.toEqual({ diff --git a/extensions/feishu/src/outbound.test.ts b/extensions/feishu/src/outbound.test.ts index 11cfc957e80..39b7c1e4a63 100644 --- a/extensions/feishu/src/outbound.test.ts +++ b/extensions/feishu/src/outbound.test.ts @@ -29,12 +29,16 @@ vi.mock("./runtime.js", () => ({ import { feishuOutbound } from "./outbound.js"; const sendText = feishuOutbound.sendText!; +function resetOutboundMocks() { + vi.clearAllMocks(); + sendMessageFeishuMock.mockResolvedValue({ messageId: "text_msg" }); + sendMarkdownCardFeishuMock.mockResolvedValue({ messageId: "card_msg" }); + sendMediaFeishuMock.mockResolvedValue({ messageId: "media_msg" }); +} + describe("feishuOutbound.sendText local-image auto-convert", () => { beforeEach(() => { - vi.clearAllMocks(); - sendMessageFeishuMock.mockResolvedValue({ messageId: "text_msg" }); - sendMarkdownCardFeishuMock.mockResolvedValue({ messageId: "card_msg" }); - sendMediaFeishuMock.mockResolvedValue({ messageId: "media_msg" }); + resetOutboundMocks(); }); async function createTmpImage(ext = ".png"): Promise<{ dir: string; file: string }> { @@ -181,10 +185,7 @@ describe("feishuOutbound.sendText local-image auto-convert", () => { describe("feishuOutbound.sendText replyToId forwarding", () => { beforeEach(() => { - vi.clearAllMocks(); - sendMessageFeishuMock.mockResolvedValue({ messageId: "text_msg" }); - sendMarkdownCardFeishuMock.mockResolvedValue({ messageId: "card_msg" }); - sendMediaFeishuMock.mockResolvedValue({ messageId: "media_msg" }); + resetOutboundMocks(); }); it("forwards replyToId as replyToMessageId to sendMessageFeishu", async () => { @@ -249,10 +250,7 @@ describe("feishuOutbound.sendText replyToId forwarding", () => { describe("feishuOutbound.sendMedia replyToId forwarding", () => { beforeEach(() => { - vi.clearAllMocks(); - sendMessageFeishuMock.mockResolvedValue({ messageId: "text_msg" }); - sendMarkdownCardFeishuMock.mockResolvedValue({ messageId: "card_msg" }); - sendMediaFeishuMock.mockResolvedValue({ messageId: "media_msg" }); + resetOutboundMocks(); }); it("forwards replyToId to sendMediaFeishu", async () => { @@ -292,10 +290,7 @@ describe("feishuOutbound.sendMedia replyToId forwarding", () => { describe("feishuOutbound.sendMedia renderMode", () => { beforeEach(() => { - vi.clearAllMocks(); - sendMessageFeishuMock.mockResolvedValue({ messageId: "text_msg" }); - sendMarkdownCardFeishuMock.mockResolvedValue({ messageId: "card_msg" }); - sendMediaFeishuMock.mockResolvedValue({ messageId: "media_msg" }); + resetOutboundMocks(); }); it("uses markdown cards for captions when renderMode=card", async () => { diff --git a/extensions/feishu/src/reply-dispatcher.test.ts b/extensions/feishu/src/reply-dispatcher.test.ts index e6f7fd4d974..10b829857a1 100644 --- a/extensions/feishu/src/reply-dispatcher.test.ts +++ b/extensions/feishu/src/reply-dispatcher.test.ts @@ -25,40 +25,27 @@ vi.mock("./typing.js", () => ({ addTypingIndicator: addTypingIndicatorMock, removeTypingIndicator: removeTypingIndicatorMock, })); -vi.mock("./streaming-card.js", () => ({ - mergeStreamingText: (previousText: string | undefined, nextText: string | undefined) => { - const previous = typeof previousText === "string" ? previousText : ""; - const next = typeof nextText === "string" ? nextText : ""; - if (!next) { - return previous; - } - if (!previous || next === previous) { - return next; - } - if (next.startsWith(previous)) { - return next; - } - if (previous.startsWith(next)) { - return previous; - } - return `${previous}${next}`; - }, - FeishuStreamingSession: class { - active = false; - start = vi.fn(async () => { - this.active = true; - }); - update = vi.fn(async () => {}); - close = vi.fn(async () => { - this.active = false; - }); - isActive = vi.fn(() => this.active); +vi.mock("./streaming-card.js", async () => { + const actual = await vi.importActual("./streaming-card.js"); + return { + mergeStreamingText: actual.mergeStreamingText, + FeishuStreamingSession: class { + active = false; + start = vi.fn(async () => { + this.active = true; + }); + update = vi.fn(async () => {}); + close = vi.fn(async () => { + this.active = false; + }); + isActive = vi.fn(() => this.active); - constructor() { - streamingInstances.push(this); - } - }, -})); + constructor() { + streamingInstances.push(this); + } + }, + }; +}); import { createFeishuReplyDispatcher } from "./reply-dispatcher.js"; diff --git a/extensions/google-gemini-cli-auth/oauth.test.ts b/extensions/google-gemini-cli-auth/oauth.test.ts index 1471f804771..02100b73b1f 100644 --- a/extensions/google-gemini-cli-auth/oauth.test.ts +++ b/extensions/google-gemini-cli-auth/oauth.test.ts @@ -144,6 +144,13 @@ describe("extractGeminiCliCredentials", () => { } } + function expectFakeCliCredentials(result: unknown) { + expect(result).toEqual({ + clientId: FAKE_CLIENT_ID, + clientSecret: FAKE_CLIENT_SECRET, + }); + } + beforeEach(async () => { vi.clearAllMocks(); originalPath = process.env.PATH; @@ -169,10 +176,7 @@ describe("extractGeminiCliCredentials", () => { clearCredentialsCache(); const result = extractGeminiCliCredentials(); - expect(result).toEqual({ - clientId: FAKE_CLIENT_ID, - clientSecret: FAKE_CLIENT_SECRET, - }); + expectFakeCliCredentials(result); }); it("extracts credentials when PATH entry is an npm global shim", async () => { @@ -182,10 +186,7 @@ describe("extractGeminiCliCredentials", () => { clearCredentialsCache(); const result = extractGeminiCliCredentials(); - expect(result).toEqual({ - clientId: FAKE_CLIENT_ID, - clientSecret: FAKE_CLIENT_SECRET, - }); + expectFakeCliCredentials(result); }); it("returns null when oauth2.js cannot be found", async () => { @@ -274,16 +275,16 @@ describe("loginGeminiCliOAuth", () => { }); } - async function runRemoteLoginWithCapturedAuthUrl( - loginGeminiCliOAuth: (options: { - isRemote: boolean; - openUrl: () => Promise; - log: (msg: string) => void; - note: () => Promise; - prompt: () => Promise; - progress: { update: () => void; stop: () => void }; - }) => Promise<{ projectId: string }>, - ) { + type LoginGeminiCliOAuthFn = (options: { + isRemote: boolean; + openUrl: () => Promise; + log: (msg: string) => void; + note: () => Promise; + prompt: () => Promise; + progress: { update: () => void; stop: () => void }; + }) => Promise<{ projectId: string }>; + + async function runRemoteLoginWithCapturedAuthUrl(loginGeminiCliOAuth: LoginGeminiCliOAuthFn) { let authUrl = ""; const result = await loginGeminiCliOAuth({ isRemote: true, @@ -304,6 +305,14 @@ describe("loginGeminiCliOAuth", () => { return { result, authUrl }; } + async function runRemoteLoginExpectingProjectId( + loginGeminiCliOAuth: LoginGeminiCliOAuthFn, + projectId: string, + ) { + const { result } = await runRemoteLoginWithCapturedAuthUrl(loginGeminiCliOAuth); + expect(result.projectId).toBe(projectId); + } + let envSnapshot: Partial>; beforeEach(() => { envSnapshot = Object.fromEntries(ENV_KEYS.map((key) => [key, process.env[key]])); @@ -357,9 +366,7 @@ describe("loginGeminiCliOAuth", () => { vi.stubGlobal("fetch", fetchMock); const { loginGeminiCliOAuth } = await import("./oauth.js"); - const { result } = await runRemoteLoginWithCapturedAuthUrl(loginGeminiCliOAuth); - - expect(result.projectId).toBe("daily-project"); + await runRemoteLoginExpectingProjectId(loginGeminiCliOAuth, "daily-project"); const loadRequests = requests.filter((request) => request.url.includes("v1internal:loadCodeAssist"), ); @@ -414,9 +421,7 @@ describe("loginGeminiCliOAuth", () => { vi.stubGlobal("fetch", fetchMock); const { loginGeminiCliOAuth } = await import("./oauth.js"); - const { result } = await runRemoteLoginWithCapturedAuthUrl(loginGeminiCliOAuth); - - expect(result.projectId).toBe("env-project"); + await runRemoteLoginExpectingProjectId(loginGeminiCliOAuth, "env-project"); expect(requests.filter((url) => url.includes("v1internal:loadCodeAssist"))).toHaveLength(3); expect(requests.some((url) => url.includes("v1internal:onboardUser"))).toBe(false); }); diff --git a/extensions/line/src/channel.ts b/extensions/line/src/channel.ts index ddc612b8fa7..982d7670082 100644 --- a/extensions/line/src/channel.ts +++ b/extensions/line/src/channel.ts @@ -347,6 +347,16 @@ export const linePlugin: ChannelPlugin = { : []; const mediaUrls = payload.mediaUrls ?? (payload.mediaUrl ? [payload.mediaUrl] : []); const shouldSendQuickRepliesInline = chunks.length === 0 && hasQuickReplies; + const sendMediaMessages = async () => { + for (const url of mediaUrls) { + lastResult = await runtime.channel.line.sendMessageLine(to, "", { + verbose: false, + mediaUrl: url, + cfg, + accountId: accountId ?? undefined, + }); + } + }; if (!shouldSendQuickRepliesInline) { if (lineData.flexMessage) { @@ -391,14 +401,7 @@ export const linePlugin: ChannelPlugin = { const sendMediaAfterText = !(hasQuickReplies && chunks.length > 0); if (mediaUrls.length > 0 && !shouldSendQuickRepliesInline && !sendMediaAfterText) { - for (const url of mediaUrls) { - lastResult = await runtime.channel.line.sendMessageLine(to, "", { - verbose: false, - mediaUrl: url, - cfg, - accountId: accountId ?? undefined, - }); - } + await sendMediaMessages(); } if (chunks.length > 0) { @@ -471,14 +474,7 @@ export const linePlugin: ChannelPlugin = { } if (mediaUrls.length > 0 && !shouldSendQuickRepliesInline && sendMediaAfterText) { - for (const url of mediaUrls) { - lastResult = await runtime.channel.line.sendMessageLine(to, "", { - verbose: false, - mediaUrl: url, - cfg, - accountId: accountId ?? undefined, - }); - } + await sendMediaMessages(); } if (lastResult) { diff --git a/extensions/llm-task/src/llm-task-tool.test.ts b/extensions/llm-task/src/llm-task-tool.test.ts index fc9f0e07215..2bf0cb655aa 100644 --- a/extensions/llm-task/src/llm-task-tool.test.ts +++ b/extensions/llm-task/src/llm-task-tool.test.ts @@ -29,6 +29,21 @@ function fakeApi(overrides: any = {}) { }; } +function mockEmbeddedRunJson(payload: unknown) { + // oxlint-disable-next-line typescript/no-explicit-any + (runEmbeddedPiAgent as any).mockResolvedValueOnce({ + meta: {}, + payloads: [{ text: JSON.stringify(payload) }], + }); +} + +async function executeEmbeddedRun(input: Record) { + const tool = createLlmTaskTool(fakeApi()); + await tool.execute("id", input); + // oxlint-disable-next-line typescript/no-explicit-any + return (runEmbeddedPiAgent as any).mock.calls[0]?.[0]; +} + describe("llm-task tool (json-only)", () => { beforeEach(() => vi.clearAllMocks()); @@ -96,42 +111,25 @@ describe("llm-task tool (json-only)", () => { }); it("passes provider/model overrides to embedded runner", async () => { - // oxlint-disable-next-line typescript/no-explicit-any - (runEmbeddedPiAgent as any).mockResolvedValueOnce({ - meta: {}, - payloads: [{ text: JSON.stringify({ ok: true }) }], + mockEmbeddedRunJson({ ok: true }); + const call = await executeEmbeddedRun({ + prompt: "x", + provider: "anthropic", + model: "claude-4-sonnet", }); - const tool = createLlmTaskTool(fakeApi()); - await tool.execute("id", { prompt: "x", provider: "anthropic", model: "claude-4-sonnet" }); - // oxlint-disable-next-line typescript/no-explicit-any - const call = (runEmbeddedPiAgent as any).mock.calls[0]?.[0]; expect(call.provider).toBe("anthropic"); expect(call.model).toBe("claude-4-sonnet"); }); it("passes thinking override to embedded runner", async () => { - // oxlint-disable-next-line typescript/no-explicit-any - (runEmbeddedPiAgent as any).mockResolvedValueOnce({ - meta: {}, - payloads: [{ text: JSON.stringify({ ok: true }) }], - }); - const tool = createLlmTaskTool(fakeApi()); - await tool.execute("id", { prompt: "x", thinking: "high" }); - // oxlint-disable-next-line typescript/no-explicit-any - const call = (runEmbeddedPiAgent as any).mock.calls[0]?.[0]; + mockEmbeddedRunJson({ ok: true }); + const call = await executeEmbeddedRun({ prompt: "x", thinking: "high" }); expect(call.thinkLevel).toBe("high"); }); it("normalizes thinking aliases", async () => { - // oxlint-disable-next-line typescript/no-explicit-any - (runEmbeddedPiAgent as any).mockResolvedValueOnce({ - meta: {}, - payloads: [{ text: JSON.stringify({ ok: true }) }], - }); - const tool = createLlmTaskTool(fakeApi()); - await tool.execute("id", { prompt: "x", thinking: "on" }); - // oxlint-disable-next-line typescript/no-explicit-any - const call = (runEmbeddedPiAgent as any).mock.calls[0]?.[0]; + mockEmbeddedRunJson({ ok: true }); + const call = await executeEmbeddedRun({ prompt: "x", thinking: "on" }); expect(call.thinkLevel).toBe("low"); }); @@ -150,24 +148,13 @@ describe("llm-task tool (json-only)", () => { }); it("does not pass thinkLevel when thinking is omitted", async () => { - // oxlint-disable-next-line typescript/no-explicit-any - (runEmbeddedPiAgent as any).mockResolvedValueOnce({ - meta: {}, - payloads: [{ text: JSON.stringify({ ok: true }) }], - }); - const tool = createLlmTaskTool(fakeApi()); - await tool.execute("id", { prompt: "x" }); - // oxlint-disable-next-line typescript/no-explicit-any - const call = (runEmbeddedPiAgent as any).mock.calls[0]?.[0]; + mockEmbeddedRunJson({ ok: true }); + const call = await executeEmbeddedRun({ prompt: "x" }); expect(call.thinkLevel).toBeUndefined(); }); it("enforces allowedModels", async () => { - // oxlint-disable-next-line typescript/no-explicit-any - (runEmbeddedPiAgent as any).mockResolvedValueOnce({ - meta: {}, - payloads: [{ text: JSON.stringify({ ok: true }) }], - }); + mockEmbeddedRunJson({ ok: true }); const tool = createLlmTaskTool( fakeApi({ pluginConfig: { allowedModels: ["openai-codex/gpt-5.2"] } }), ); @@ -177,15 +164,8 @@ describe("llm-task tool (json-only)", () => { }); it("disables tools for embedded run", async () => { - // oxlint-disable-next-line typescript/no-explicit-any - (runEmbeddedPiAgent as any).mockResolvedValueOnce({ - meta: {}, - payloads: [{ text: JSON.stringify({ ok: true }) }], - }); - const tool = createLlmTaskTool(fakeApi()); - await tool.execute("id", { prompt: "x" }); - // oxlint-disable-next-line typescript/no-explicit-any - const call = (runEmbeddedPiAgent as any).mock.calls[0]?.[0]; + mockEmbeddedRunJson({ ok: true }); + const call = await executeEmbeddedRun({ prompt: "x" }); expect(call.disableTools).toBe(true); }); }); diff --git a/extensions/matrix/src/channel.directory.test.ts b/extensions/matrix/src/channel.directory.test.ts index 51c781c0b75..71c9f1c31b1 100644 --- a/extensions/matrix/src/channel.directory.test.ts +++ b/extensions/matrix/src/channel.directory.test.ts @@ -2,26 +2,12 @@ import type { PluginRuntime, RuntimeEnv } from "openclaw/plugin-sdk/matrix"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { matrixPlugin } from "./channel.js"; import { setMatrixRuntime } from "./runtime.js"; +import { createMatrixBotSdkMock } from "./test-mocks.js"; import type { CoreConfig } from "./types.js"; -vi.mock("@vector-im/matrix-bot-sdk", () => ({ - ConsoleLogger: class { - trace = vi.fn(); - debug = vi.fn(); - info = vi.fn(); - warn = vi.fn(); - error = vi.fn(); - }, - MatrixClient: class {}, - LogService: { - setLogger: vi.fn(), - warn: vi.fn(), - info: vi.fn(), - debug: vi.fn(), - }, - SimpleFsStorageProvider: class {}, - RustSdkCryptoStorageProvider: class {}, -})); +vi.mock("@vector-im/matrix-bot-sdk", () => + createMatrixBotSdkMock({ includeVerboseLogService: true }), +); describe("matrix directory", () => { const runtimeEnv: RuntimeEnv = { diff --git a/extensions/matrix/src/matrix/send.test.ts b/extensions/matrix/src/matrix/send.test.ts index dabe915b388..2bf21023909 100644 --- a/extensions/matrix/src/matrix/send.test.ts +++ b/extensions/matrix/src/matrix/send.test.ts @@ -1,6 +1,7 @@ import type { PluginRuntime } from "openclaw/plugin-sdk/matrix"; import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { setMatrixRuntime } from "../runtime.js"; +import { createMatrixBotSdkMock } from "../test-mocks.js"; vi.mock("music-metadata", () => ({ // `resolveMediaDurationMs` lazily imports `music-metadata`; in tests we don't @@ -8,21 +9,13 @@ vi.mock("music-metadata", () => ({ parseBuffer: vi.fn().mockResolvedValue({ format: {} }), })); -vi.mock("@vector-im/matrix-bot-sdk", () => ({ - ConsoleLogger: class { - trace = vi.fn(); - debug = vi.fn(); - info = vi.fn(); - warn = vi.fn(); - error = vi.fn(); - }, - LogService: { - setLogger: vi.fn(), - }, - MatrixClient: vi.fn(), - SimpleFsStorageProvider: vi.fn(), - RustSdkCryptoStorageProvider: vi.fn(), -})); +vi.mock("@vector-im/matrix-bot-sdk", () => + createMatrixBotSdkMock({ + matrixClient: vi.fn(), + simpleFsStorageProvider: vi.fn(), + rustSdkCryptoStorageProvider: vi.fn(), + }), +); vi.mock("./send-queue.js", () => ({ enqueueSend: async (_roomId: string, fn: () => Promise) => await fn(), diff --git a/extensions/matrix/src/resolve-targets.test.ts b/extensions/matrix/src/resolve-targets.test.ts index 10dff313a2e..02a5088e8ae 100644 --- a/extensions/matrix/src/resolve-targets.test.ts +++ b/extensions/matrix/src/resolve-targets.test.ts @@ -8,6 +8,15 @@ vi.mock("./directory-live.js", () => ({ listMatrixDirectoryGroupsLive: vi.fn(), })); +async function resolveUserTarget(input = "Alice") { + const [result] = await resolveMatrixTargets({ + cfg: {}, + inputs: [input], + kind: "user", + }); + return result; +} + describe("resolveMatrixTargets (users)", () => { beforeEach(() => { vi.mocked(listMatrixDirectoryPeersLive).mockReset(); @@ -20,11 +29,7 @@ describe("resolveMatrixTargets (users)", () => { ]; vi.mocked(listMatrixDirectoryPeersLive).mockResolvedValue(matches); - const [result] = await resolveMatrixTargets({ - cfg: {}, - inputs: ["Alice"], - kind: "user", - }); + const result = await resolveUserTarget(); expect(result?.resolved).toBe(true); expect(result?.id).toBe("@alice:example.org"); @@ -37,11 +42,7 @@ describe("resolveMatrixTargets (users)", () => { ]; vi.mocked(listMatrixDirectoryPeersLive).mockResolvedValue(matches); - const [result] = await resolveMatrixTargets({ - cfg: {}, - inputs: ["Alice"], - kind: "user", - }); + const result = await resolveUserTarget(); expect(result?.resolved).toBe(false); expect(result?.note).toMatch(/use full Matrix ID/i); diff --git a/extensions/matrix/src/test-mocks.ts b/extensions/matrix/src/test-mocks.ts new file mode 100644 index 00000000000..8a104b94650 --- /dev/null +++ b/extensions/matrix/src/test-mocks.ts @@ -0,0 +1,33 @@ +import { vi } from "vitest"; + +type MatrixBotSdkMockParams = { + matrixClient?: unknown; + simpleFsStorageProvider?: unknown; + rustSdkCryptoStorageProvider?: unknown; + includeVerboseLogService?: boolean; +}; + +export function createMatrixBotSdkMock(params: MatrixBotSdkMockParams = {}) { + return { + ConsoleLogger: class { + trace = vi.fn(); + debug = vi.fn(); + info = vi.fn(); + warn = vi.fn(); + error = vi.fn(); + }, + MatrixClient: params.matrixClient ?? class {}, + LogService: { + setLogger: vi.fn(), + ...(params.includeVerboseLogService + ? { + warn: vi.fn(), + info: vi.fn(), + debug: vi.fn(), + } + : {}), + }, + SimpleFsStorageProvider: params.simpleFsStorageProvider ?? class {}, + RustSdkCryptoStorageProvider: params.rustSdkCryptoStorageProvider ?? class {}, + }; +} diff --git a/extensions/memory-lancedb/index.test.ts b/extensions/memory-lancedb/index.test.ts index 2d9a6db1063..a733c3dffb8 100644 --- a/extensions/memory-lancedb/index.test.ts +++ b/extensions/memory-lancedb/index.test.ts @@ -18,12 +18,12 @@ const HAS_OPENAI_KEY = Boolean(process.env.OPENAI_API_KEY); const liveEnabled = HAS_OPENAI_KEY && process.env.OPENCLAW_LIVE_TEST === "1"; const describeLive = liveEnabled ? describe : describe.skip; -describe("memory plugin e2e", () => { - let tmpDir: string; - let dbPath: string; +function installTmpDirHarness(params: { prefix: string }) { + let tmpDir = ""; + let dbPath = ""; beforeEach(async () => { - tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-memory-test-")); + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), params.prefix)); dbPath = path.join(tmpDir, "lancedb"); }); @@ -33,6 +33,27 @@ describe("memory plugin e2e", () => { } }); + return { + getTmpDir: () => tmpDir, + getDbPath: () => dbPath, + }; +} + +describe("memory plugin e2e", () => { + const { getDbPath } = installTmpDirHarness({ prefix: "openclaw-memory-test-" }); + + async function parseConfig(overrides: Record = {}) { + const { default: memoryPlugin } = await import("./index.js"); + return memoryPlugin.configSchema?.parse?.({ + embedding: { + apiKey: OPENAI_API_KEY, + model: "text-embedding-3-small", + }, + dbPath: getDbPath(), + ...overrides, + }); + } + test("memory plugin registers and initializes correctly", async () => { // Dynamic import to avoid loading LanceDB when not testing const { default: memoryPlugin } = await import("./index.js"); @@ -46,21 +67,14 @@ describe("memory plugin e2e", () => { }); test("config schema parses valid config", async () => { - const { default: memoryPlugin } = await import("./index.js"); - - const config = memoryPlugin.configSchema?.parse?.({ - embedding: { - apiKey: OPENAI_API_KEY, - model: "text-embedding-3-small", - }, - dbPath, + const config = await parseConfig({ autoCapture: true, autoRecall: true, }); expect(config).toBeDefined(); expect(config?.embedding?.apiKey).toBe(OPENAI_API_KEY); - expect(config?.dbPath).toBe(dbPath); + expect(config?.dbPath).toBe(getDbPath()); expect(config?.captureMaxChars).toBe(500); }); @@ -74,7 +88,7 @@ describe("memory plugin e2e", () => { embedding: { apiKey: "${TEST_MEMORY_API_KEY}", }, - dbPath, + dbPath: getDbPath(), }); expect(config?.embedding?.apiKey).toBe("test-key-123"); @@ -88,7 +102,7 @@ describe("memory plugin e2e", () => { expect(() => { memoryPlugin.configSchema?.parse?.({ embedding: {}, - dbPath, + dbPath: getDbPath(), }); }).toThrow("embedding.apiKey is required"); }); @@ -99,21 +113,14 @@ describe("memory plugin e2e", () => { expect(() => { memoryPlugin.configSchema?.parse?.({ embedding: { apiKey: OPENAI_API_KEY }, - dbPath, + dbPath: getDbPath(), captureMaxChars: 99, }); }).toThrow("captureMaxChars must be between 100 and 10000"); }); test("config schema accepts captureMaxChars override", async () => { - const { default: memoryPlugin } = await import("./index.js"); - - const config = memoryPlugin.configSchema?.parse?.({ - embedding: { - apiKey: OPENAI_API_KEY, - model: "text-embedding-3-small", - }, - dbPath, + const config = await parseConfig({ captureMaxChars: 1800, }); @@ -121,15 +128,7 @@ describe("memory plugin e2e", () => { }); test("config schema keeps autoCapture disabled by default", async () => { - const { default: memoryPlugin } = await import("./index.js"); - - const config = memoryPlugin.configSchema?.parse?.({ - embedding: { - apiKey: OPENAI_API_KEY, - model: "text-embedding-3-small", - }, - dbPath, - }); + const config = await parseConfig(); expect(config?.autoCapture).toBe(false); expect(config?.autoRecall).toBe(true); @@ -176,7 +175,7 @@ describe("memory plugin e2e", () => { model: "text-embedding-3-small", dimensions: 1024, }, - dbPath, + dbPath: getDbPath(), autoCapture: false, autoRecall: false, }, @@ -279,19 +278,7 @@ describe("memory plugin e2e", () => { // Live tests that require OpenAI API key and actually use LanceDB describeLive("memory plugin live tests", () => { - let tmpDir: string; - let dbPath: string; - - beforeEach(async () => { - tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-memory-live-")); - dbPath = path.join(tmpDir, "lancedb"); - }); - - afterEach(async () => { - if (tmpDir) { - await fs.rm(tmpDir, { recursive: true, force: true }); - } - }); + const { getDbPath } = installTmpDirHarness({ prefix: "openclaw-memory-live-" }); test("memory tools work end-to-end", async () => { const { default: memoryPlugin } = await import("./index.js"); @@ -318,7 +305,7 @@ describeLive("memory plugin live tests", () => { apiKey: liveApiKey, model: "text-embedding-3-small", }, - dbPath, + dbPath: getDbPath(), autoCapture: false, autoRecall: false, }, diff --git a/extensions/msteams/src/attachments.test.ts b/extensions/msteams/src/attachments.test.ts index 6887fad7fcb..790dc8bd33f 100644 --- a/extensions/msteams/src/attachments.test.ts +++ b/extensions/msteams/src/attachments.test.ts @@ -88,14 +88,17 @@ function isUrlAllowedBySsrfPolicy(url: string, policy?: SsrFPolicy): boolean { ); } -const fetchRemoteMediaMock = vi.fn(async (params: RemoteMediaFetchParams) => { +async function fetchRemoteMediaWithRedirects( + params: RemoteMediaFetchParams, + requestInit?: RequestInit, +) { const fetchFn = params.fetchImpl ?? fetch; let currentUrl = params.url; for (let i = 0; i <= MAX_REDIRECT_HOPS; i += 1) { if (!isUrlAllowedBySsrfPolicy(currentUrl, params.ssrfPolicy)) { throw new Error(`Blocked hostname (not in allowlist): ${currentUrl}`); } - const res = await fetchFn(currentUrl, { redirect: "manual" }); + const res = await fetchFn(currentUrl, { redirect: "manual", ...requestInit }); if (REDIRECT_STATUS_CODES.includes(res.status)) { const location = res.headers.get("location"); if (!location) { @@ -107,6 +110,10 @@ const fetchRemoteMediaMock = vi.fn(async (params: RemoteMediaFetchParams) => { return readRemoteMediaResponse(res, params); } throw new Error("too many redirects"); +} + +const fetchRemoteMediaMock = vi.fn(async (params: RemoteMediaFetchParams) => { + return await fetchRemoteMediaWithRedirects(params); }); const runtimeStub: PluginRuntime = createPluginRuntimeMock({ @@ -720,24 +727,9 @@ describe("msteams attachments", () => { }); fetchRemoteMediaMock.mockImplementationOnce(async (params) => { - const fetchFn = params.fetchImpl ?? fetch; - let currentUrl = params.url; - for (let i = 0; i < MAX_REDIRECT_HOPS; i += 1) { - const res = await fetchFn(currentUrl, { - redirect: "manual", - dispatcher: {}, - } as RequestInit); - if (REDIRECT_STATUS_CODES.includes(res.status)) { - const location = res.headers.get("location"); - if (!location) { - throw new Error("redirect missing location"); - } - currentUrl = new URL(location, currentUrl).toString(); - continue; - } - return readRemoteMediaResponse(res, params); - } - throw new Error("too many redirects"); + return await fetchRemoteMediaWithRedirects(params, { + dispatcher: {}, + } as RequestInit); }); const media = await downloadAttachmentsWithFetch( diff --git a/extensions/msteams/src/messenger.test.ts b/extensions/msteams/src/messenger.test.ts index aa0a92b5159..cc4cf2fb6f0 100644 --- a/extensions/msteams/src/messenger.test.ts +++ b/extensions/msteams/src/messenger.test.ts @@ -139,6 +139,22 @@ describe("msteams messenger", () => { }); describe("sendMSTeamsMessages", () => { + function createRevokedThreadContext(params?: { failAfterAttempt?: number; sent?: string[] }) { + let attempt = 0; + return { + sendActivity: async (activity: unknown) => { + const { text } = activity as { text?: string }; + const content = text ?? ""; + attempt += 1; + if (params?.failAfterAttempt && attempt < params.failAfterAttempt) { + params.sent?.push(content); + return { id: `id:${content}` }; + } + throw new TypeError(REVOCATION_ERROR); + }, + }; + } + const baseRef: StoredConversationReference = { activityId: "activity123", user: { id: "user123", name: "User" }, @@ -305,13 +321,7 @@ describe("msteams messenger", () => { it("falls back to proactive messaging when thread context is revoked", async () => { const proactiveSent: string[] = []; - - const ctx = { - sendActivity: async () => { - throw new TypeError(REVOCATION_ERROR); - }, - }; - + const ctx = createRevokedThreadContext(); const adapter = createFallbackAdapter(proactiveSent); const ids = await sendMSTeamsMessages({ @@ -331,21 +341,7 @@ describe("msteams messenger", () => { it("falls back only for remaining thread messages after context revocation", async () => { const threadSent: string[] = []; const proactiveSent: string[] = []; - let attempt = 0; - - const ctx = { - sendActivity: async (activity: unknown) => { - const { text } = activity as { text?: string }; - const content = text ?? ""; - attempt += 1; - if (attempt === 1) { - threadSent.push(content); - return { id: `id:${content}` }; - } - throw new TypeError(REVOCATION_ERROR); - }, - }; - + const ctx = createRevokedThreadContext({ failAfterAttempt: 2, sent: threadSent }); const adapter = createFallbackAdapter(proactiveSent); const ids = await sendMSTeamsMessages({ diff --git a/extensions/sglang/index.ts b/extensions/sglang/index.ts index 4c9102caebc..13e301de28b 100644 --- a/extensions/sglang/index.ts +++ b/extensions/sglang/index.ts @@ -2,11 +2,9 @@ import { buildSglangProvider, configureOpenAICompatibleSelfHostedProviderNonInteractive, emptyPluginConfigSchema, - promptAndConfigureOpenAICompatibleSelfHostedProvider, + promptAndConfigureOpenAICompatibleSelfHostedProviderAuth, type OpenClawPluginApi, - type ProviderAuthContext, type ProviderAuthMethodNonInteractiveContext, - type ProviderAuthResult, type ProviderDiscoveryContext, } from "openclaw/plugin-sdk/core"; @@ -30,8 +28,8 @@ const sglangPlugin = { label: "SGLang", hint: "Fast self-hosted OpenAI-compatible server", kind: "custom", - run: async (ctx: ProviderAuthContext): Promise => { - const result = await promptAndConfigureOpenAICompatibleSelfHostedProvider({ + run: (ctx) => + promptAndConfigureOpenAICompatibleSelfHostedProviderAuth({ cfg: ctx.config, prompter: ctx.prompter, providerId: PROVIDER_ID, @@ -39,18 +37,7 @@ const sglangPlugin = { defaultBaseUrl: DEFAULT_BASE_URL, defaultApiKeyEnvVar: "SGLANG_API_KEY", modelPlaceholder: "Qwen/Qwen3-8B", - }); - return { - profiles: [ - { - profileId: result.profileId, - credential: result.credential, - }, - ], - configPatch: result.config, - defaultModel: result.modelRef, - }; - }, + }), runNonInteractive: async (ctx: ProviderAuthMethodNonInteractiveContext) => configureOpenAICompatibleSelfHostedProviderNonInteractive({ ctx, diff --git a/extensions/synology-chat/src/channel.integration.test.ts b/extensions/synology-chat/src/channel.integration.test.ts index b9cb5484621..e5d1e7f24c9 100644 --- a/extensions/synology-chat/src/channel.integration.test.ts +++ b/extensions/synology-chat/src/channel.integration.test.ts @@ -1,5 +1,9 @@ import type { IncomingMessage, ServerResponse } from "node:http"; import { beforeEach, describe, expect, it, vi } from "vitest"; +import { + dispatchReplyWithBufferedBlockDispatcher, + registerPluginHttpRouteMock, +} from "./channel.test-mocks.js"; import { makeFormBody, makeReq, makeRes } from "./test-http-utils.js"; type RegisteredRoute = { @@ -8,41 +12,6 @@ type RegisteredRoute = { handler: (req: IncomingMessage, res: ServerResponse) => Promise; }; -const registerPluginHttpRouteMock = vi.fn<(params: RegisteredRoute) => () => void>(() => vi.fn()); -const dispatchReplyWithBufferedBlockDispatcher = vi.fn().mockResolvedValue({ counts: {} }); - -vi.mock("openclaw/plugin-sdk/synology-chat", async (importOriginal) => { - const actual = await importOriginal(); - return { - ...actual, - DEFAULT_ACCOUNT_ID: "default", - setAccountEnabledInConfigSection: vi.fn((_opts: any) => ({})), - registerPluginHttpRoute: registerPluginHttpRouteMock, - buildChannelConfigSchema: vi.fn((schema: any) => ({ schema })), - createFixedWindowRateLimiter: vi.fn(() => ({ - isRateLimited: vi.fn(() => false), - size: vi.fn(() => 0), - clear: vi.fn(), - })), - }; -}); - -vi.mock("./runtime.js", () => ({ - getSynologyRuntime: vi.fn(() => ({ - config: { loadConfig: vi.fn().mockResolvedValue({}) }, - channel: { - reply: { - dispatchReplyWithBufferedBlockDispatcher, - }, - }, - })), -})); - -vi.mock("./client.js", () => ({ - sendMessage: vi.fn().mockResolvedValue(true), - sendFileUrl: vi.fn().mockResolvedValue(true), -})); - const { createSynologyChatPlugin } = await import("./channel.js"); describe("Synology channel wiring integration", () => { beforeEach(() => { diff --git a/extensions/synology-chat/src/channel.test-mocks.ts b/extensions/synology-chat/src/channel.test-mocks.ts new file mode 100644 index 00000000000..b27c7ad434c --- /dev/null +++ b/extensions/synology-chat/src/channel.test-mocks.ts @@ -0,0 +1,73 @@ +import type { IncomingMessage, ServerResponse } from "node:http"; +import { vi } from "vitest"; + +export type RegisteredRoute = { + path: string; + accountId: string; + handler: (req: IncomingMessage, res: ServerResponse) => Promise; +}; + +export const registerPluginHttpRouteMock = vi.fn<(params: RegisteredRoute) => () => void>(() => + vi.fn(), +); + +export const dispatchReplyWithBufferedBlockDispatcher = vi.fn().mockResolvedValue({ counts: {} }); + +async function readRequestBodyWithLimitForTest(req: IncomingMessage): Promise { + return await new Promise((resolve, reject) => { + const chunks: Buffer[] = []; + req.on("data", (chunk) => { + chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)); + }); + req.on("end", () => resolve(Buffer.concat(chunks).toString("utf8"))); + req.on("error", reject); + }); +} + +vi.mock("openclaw/plugin-sdk/synology-chat", () => ({ + DEFAULT_ACCOUNT_ID: "default", + setAccountEnabledInConfigSection: vi.fn((_opts: unknown) => ({})), + registerPluginHttpRoute: registerPluginHttpRouteMock, + buildChannelConfigSchema: vi.fn((schema: unknown) => ({ schema })), + readRequestBodyWithLimit: vi.fn(readRequestBodyWithLimitForTest), + isRequestBodyLimitError: vi.fn(() => false), + requestBodyErrorToText: vi.fn(() => "Request body too large"), + createFixedWindowRateLimiter: vi.fn(() => ({ + isRateLimited: vi.fn(() => false), + size: vi.fn(() => 0), + clear: vi.fn(), + })), +})); + +vi.mock("./client.js", () => ({ + sendMessage: vi.fn().mockResolvedValue(true), + sendFileUrl: vi.fn().mockResolvedValue(true), +})); + +vi.mock("./runtime.js", () => ({ + getSynologyRuntime: vi.fn(() => ({ + config: { loadConfig: vi.fn().mockResolvedValue({}) }, + channel: { + reply: { + dispatchReplyWithBufferedBlockDispatcher, + }, + }, + })), +})); + +export 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, + }; +} diff --git a/extensions/synology-chat/src/channel.test.ts b/extensions/synology-chat/src/channel.test.ts index 2814d437c6b..bdce5f37d79 100644 --- a/extensions/synology-chat/src/channel.test.ts +++ b/extensions/synology-chat/src/channel.test.ts @@ -1,40 +1,10 @@ -import { describe, it, expect, vi, beforeEach } from "vitest"; - -// Mock external dependencies -vi.mock("openclaw/plugin-sdk/synology-chat", () => ({ - DEFAULT_ACCOUNT_ID: "default", - setAccountEnabledInConfigSection: vi.fn((_opts: any) => ({})), - registerPluginHttpRoute: vi.fn(() => vi.fn()), - buildChannelConfigSchema: vi.fn((schema: any) => ({ schema })), - createFixedWindowRateLimiter: vi.fn(() => ({ - isRateLimited: vi.fn(() => false), - size: vi.fn(() => 0), - clear: vi.fn(), - })), -})); - -vi.mock("./client.js", () => ({ - sendMessage: vi.fn().mockResolvedValue(true), - sendFileUrl: vi.fn().mockResolvedValue(true), -})); +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { makeSecurityAccount, registerPluginHttpRouteMock } from "./channel.test-mocks.js"; vi.mock("./webhook-handler.js", () => ({ createWebhookHandler: vi.fn(() => vi.fn()), })); -vi.mock("./runtime.js", () => ({ - getSynologyRuntime: vi.fn(() => ({ - config: { loadConfig: vi.fn().mockResolvedValue({}) }, - channel: { - reply: { - dispatchReplyWithBufferedBlockDispatcher: vi.fn().mockResolvedValue({ - counts: {}, - }), - }, - }, - })), -})); - vi.mock("zod", () => ({ z: { object: vi.fn(() => ({ @@ -44,24 +14,6 @@ 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", () => { @@ -321,7 +273,7 @@ describe("createSynologyChatPlugin", () => { }); it("startAccount refuses allowlist accounts with empty allowedUserIds", async () => { - const registerMock = vi.mocked(registerPluginHttpRoute); + const registerMock = registerPluginHttpRouteMock; registerMock.mockClear(); const plugin = createSynologyChatPlugin(); const { ctx, abortController } = makeStartAccountCtx({ @@ -341,7 +293,7 @@ describe("createSynologyChatPlugin", () => { it("deregisters stale route before re-registering same account/path", async () => { const unregisterFirst = vi.fn(); const unregisterSecond = vi.fn(); - const registerMock = vi.mocked(registerPluginHttpRoute); + const registerMock = registerPluginHttpRouteMock; registerMock.mockReturnValueOnce(unregisterFirst).mockReturnValueOnce(unregisterSecond); const plugin = createSynologyChatPlugin(); diff --git a/extensions/telegram/src/channel.ts b/extensions/telegram/src/channel.ts index 52ae2b15ea8..b5ae12fa06d 100644 --- a/extensions/telegram/src/channel.ts +++ b/extensions/telegram/src/channel.ts @@ -78,6 +78,61 @@ function formatDuplicateTelegramTokenReason(params: { ); } +type TelegramSendFn = ReturnType< + typeof getTelegramRuntime +>["channel"]["telegram"]["sendMessageTelegram"]; +type TelegramSendOptions = NonNullable[2]>; + +function buildTelegramSendOptions(params: { + cfg: OpenClawConfig; + mediaUrl?: string; + mediaLocalRoots?: readonly string[]; + accountId?: string; + replyToId?: string; + threadId?: string; + silent?: boolean; +}): TelegramSendOptions { + return { + verbose: false, + cfg: params.cfg, + ...(params.mediaUrl ? { mediaUrl: params.mediaUrl } : {}), + ...(params.mediaLocalRoots?.length ? { mediaLocalRoots: params.mediaLocalRoots } : {}), + messageThreadId: parseTelegramThreadId(params.threadId), + replyToMessageId: parseTelegramReplyToMessageId(params.replyToId), + accountId: params.accountId ?? undefined, + silent: params.silent ?? undefined, + }; +} + +async function sendTelegramOutbound(params: { + cfg: OpenClawConfig; + to: string; + text: string; + mediaUrl?: string; + mediaLocalRoots?: readonly string[]; + accountId?: string; + deps?: { sendTelegram?: TelegramSendFn }; + replyToId?: string; + threadId?: string; + silent?: boolean; +}) { + const send = + params.deps?.sendTelegram ?? getTelegramRuntime().channel.telegram.sendMessageTelegram; + return await send( + params.to, + params.text, + buildTelegramSendOptions({ + cfg: params.cfg, + mediaUrl: params.mediaUrl, + mediaLocalRoots: params.mediaLocalRoots, + accountId: params.accountId, + replyToId: params.replyToId, + threadId: params.threadId, + silent: params.silent, + }), + ); +} + const telegramMessageActions: ChannelMessageActionAdapter = { listActions: (ctx) => getTelegramRuntime().channel.telegram.messageActions?.listActions?.(ctx) ?? [], @@ -327,35 +382,31 @@ export const telegramPlugin: ChannelPlugin { const send = deps?.sendTelegram ?? getTelegramRuntime().channel.telegram.sendMessageTelegram; - const replyToMessageId = parseTelegramReplyToMessageId(replyToId); - const messageThreadId = parseTelegramThreadId(threadId); const result = await sendTelegramPayloadMessages({ send, to, payload, - baseOpts: { - verbose: false, + baseOpts: buildTelegramSendOptions({ cfg, mediaLocalRoots, - messageThreadId, - replyToMessageId, - accountId: accountId ?? undefined, - silent: silent ?? undefined, - }, + accountId, + replyToId, + threadId, + silent, + }), }); return { channel: "telegram", ...result }; }, sendText: async ({ cfg, to, text, accountId, deps, replyToId, threadId, silent }) => { - const send = deps?.sendTelegram ?? getTelegramRuntime().channel.telegram.sendMessageTelegram; - const replyToMessageId = parseTelegramReplyToMessageId(replyToId); - const messageThreadId = parseTelegramThreadId(threadId); - const result = await send(to, text, { - verbose: false, + const result = await sendTelegramOutbound({ cfg, - messageThreadId, - replyToMessageId, - accountId: accountId ?? undefined, - silent: silent ?? undefined, + to, + text, + accountId, + deps, + replyToId, + threadId, + silent, }); return { channel: "telegram", ...result }; }, @@ -371,18 +422,17 @@ export const telegramPlugin: ChannelPlugin { - const send = deps?.sendTelegram ?? getTelegramRuntime().channel.telegram.sendMessageTelegram; - const replyToMessageId = parseTelegramReplyToMessageId(replyToId); - const messageThreadId = parseTelegramThreadId(threadId); - const result = await send(to, text, { - verbose: false, + const result = await sendTelegramOutbound({ cfg, + to, + text, mediaUrl, mediaLocalRoots, - messageThreadId, - replyToMessageId, - accountId: accountId ?? undefined, - silent: silent ?? undefined, + accountId, + deps, + replyToId, + threadId, + silent, }); return { channel: "telegram", ...result }; }, diff --git a/extensions/thread-ownership/index.test.ts b/extensions/thread-ownership/index.test.ts index 825b4ca5bb5..3d98d8f9735 100644 --- a/extensions/thread-ownership/index.test.ts +++ b/extensions/thread-ownership/index.test.ts @@ -51,6 +51,13 @@ describe("thread-ownership plugin", () => { register(api as any); }); + async function sendSlackThreadMessage() { + return await hooks.message_sending( + { content: "hello", metadata: { threadTs: "1234.5678", channelId: "C123" }, to: "C123" }, + { channelId: "slack", conversationId: "C123" }, + ); + } + it("allows non-slack channels", async () => { const result = await hooks.message_sending( { content: "hello", metadata: { threadTs: "1234.5678", channelId: "C123" }, to: "C123" }, @@ -76,10 +83,7 @@ describe("thread-ownership plugin", () => { new Response(JSON.stringify({ owner: "test-agent" }), { status: 200 }), ); - const result = await hooks.message_sending( - { content: "hello", metadata: { threadTs: "1234.5678", channelId: "C123" }, to: "C123" }, - { channelId: "slack", conversationId: "C123" }, - ); + const result = await sendSlackThreadMessage(); expect(result).toBeUndefined(); expect(globalThis.fetch).toHaveBeenCalledWith( @@ -96,10 +100,7 @@ describe("thread-ownership plugin", () => { new Response(JSON.stringify({ owner: "other-agent" }), { status: 409 }), ); - const result = await hooks.message_sending( - { content: "hello", metadata: { threadTs: "1234.5678", channelId: "C123" }, to: "C123" }, - { channelId: "slack", conversationId: "C123" }, - ); + const result = await sendSlackThreadMessage(); expect(result).toEqual({ cancel: true }); expect(api.logger.info).toHaveBeenCalledWith(expect.stringContaining("cancelled send")); @@ -108,10 +109,7 @@ describe("thread-ownership plugin", () => { it("fails open on network error", async () => { vi.mocked(globalThis.fetch).mockRejectedValue(new Error("ECONNREFUSED")); - const result = await hooks.message_sending( - { content: "hello", metadata: { threadTs: "1234.5678", channelId: "C123" }, to: "C123" }, - { channelId: "slack", conversationId: "C123" }, - ); + const result = await sendSlackThreadMessage(); expect(result).toBeUndefined(); expect(api.logger.warn).toHaveBeenCalledWith( diff --git a/extensions/tlon/src/channel.ts b/extensions/tlon/src/channel.ts index 3c5bedbf841..b84679e1f39 100644 --- a/extensions/tlon/src/channel.ts +++ b/extensions/tlon/src/channel.ts @@ -153,6 +153,48 @@ function applyTlonSetupConfig(params: { }; } +type ResolvedTlonAccount = ReturnType; + +function resolveOutboundContext(params: { cfg: OpenClawConfig; accountId?: string; to: string }) { + const account = resolveTlonAccount(params.cfg, params.accountId ?? undefined); + if (!account.configured || !account.ship || !account.url || !account.code) { + throw new Error("Tlon account not configured"); + } + + const parsed = parseTlonTarget(params.to); + if (!parsed) { + throw new Error(`Invalid Tlon target. Use ${formatTargetHint()}`); + } + + return { account, parsed }; +} + +function resolveReplyId(replyToId?: string, threadId?: string) { + return (replyToId ?? threadId) ? String(replyToId ?? threadId) : undefined; +} + +async function withHttpPokeAccountApi( + account: ResolvedTlonAccount & { ship: string; url: string; code: string }, + run: (api: Awaited>) => Promise, +) { + const api = await createHttpPokeApi({ + url: account.url, + ship: account.ship, + code: account.code, + allowPrivateNetwork: account.allowPrivateNetwork ?? undefined, + }); + + try { + return await run(api); + } finally { + try { + await api.delete(); + } catch { + // ignore cleanup errors + } + } +} + const tlonOutbound: ChannelOutboundAdapter = { deliveryMode: "direct", textChunkLimit: 10000, @@ -170,25 +212,8 @@ const tlonOutbound: ChannelOutboundAdapter = { return { ok: true, to: parsed.nest }; }, sendText: async ({ cfg, to, text, accountId, replyToId, threadId }) => { - const account = resolveTlonAccount(cfg, accountId ?? undefined); - if (!account.configured || !account.ship || !account.url || !account.code) { - throw new Error("Tlon account not configured"); - } - - const parsed = parseTlonTarget(to); - if (!parsed) { - throw new Error(`Invalid Tlon target. Use ${formatTargetHint()}`); - } - - // Use HTTP-only poke (no EventSource) to avoid conflicts with monitor's SSE connection - const api = await createHttpPokeApi({ - url: account.url, - ship: account.ship, - code: account.code, - allowPrivateNetwork: account.allowPrivateNetwork ?? undefined, - }); - - try { + const { account, parsed } = resolveOutboundContext({ cfg, accountId, to }); + return withHttpPokeAccountApi(account, async (api) => { const fromShip = normalizeShip(account.ship); if (parsed.kind === "dm") { return await sendDm({ @@ -198,33 +223,18 @@ const tlonOutbound: ChannelOutboundAdapter = { text, }); } - const replyId = (replyToId ?? threadId) ? String(replyToId ?? threadId) : undefined; return await sendGroupMessage({ api, fromShip, hostShip: parsed.hostShip, channelName: parsed.channelName, text, - replyToId: replyId, + replyToId: resolveReplyId(replyToId, threadId), }); - } finally { - try { - await api.delete(); - } catch { - // ignore cleanup errors - } - } + }); }, sendMedia: async ({ cfg, to, text, mediaUrl, accountId, replyToId, threadId }) => { - const account = resolveTlonAccount(cfg, accountId ?? undefined); - if (!account.configured || !account.ship || !account.url || !account.code) { - throw new Error("Tlon account not configured"); - } - - const parsed = parseTlonTarget(to); - if (!parsed) { - throw new Error(`Invalid Tlon target. Use ${formatTargetHint()}`); - } + const { account, parsed } = resolveOutboundContext({ cfg, accountId, to }); // Configure the API client for uploads configureClient({ @@ -235,15 +245,7 @@ const tlonOutbound: ChannelOutboundAdapter = { }); const uploadedUrl = mediaUrl ? await uploadImageFromUrl(mediaUrl) : undefined; - - const api = await createHttpPokeApi({ - url: account.url, - ship: account.ship, - code: account.code, - allowPrivateNetwork: account.allowPrivateNetwork ?? undefined, - }); - - try { + return withHttpPokeAccountApi(account, async (api) => { const fromShip = normalizeShip(account.ship); const story = buildMediaStory(text, uploadedUrl); @@ -255,22 +257,15 @@ const tlonOutbound: ChannelOutboundAdapter = { story, }); } - const replyId = (replyToId ?? threadId) ? String(replyToId ?? threadId) : undefined; return await sendGroupMessageWithStory({ api, fromShip, hostShip: parsed.hostShip, channelName: parsed.channelName, story, - replyToId: replyId, + replyToId: resolveReplyId(replyToId, threadId), }); - } finally { - try { - await api.delete(); - } catch { - // ignore cleanup errors - } - } + }); }, }; diff --git a/extensions/tlon/src/urbit/sse-client.ts b/extensions/tlon/src/urbit/sse-client.ts index ab12977d0e8..afa87502320 100644 --- a/extensions/tlon/src/urbit/sse-client.ts +++ b/extensions/tlon/src/urbit/sse-client.ts @@ -115,20 +115,7 @@ export class UrbitSSEClient { app: string; path: string; }) { - const { response, release } = await urbitFetch({ - baseUrl: this.url, - path: `/~/channel/${this.channelId}`, - init: { - method: "PUT", - headers: { - "Content-Type": "application/json", - Cookie: this.cookie, - }, - body: JSON.stringify([subscription]), - }, - ssrfPolicy: this.ssrfPolicy, - lookupFn: this.lookupFn, - fetchImpl: this.fetchImpl, + const { response, release } = await this.putChannelPayload([subscription], { timeoutMs: 30_000, auditContext: "tlon-urbit-subscribe", }); @@ -359,20 +346,7 @@ export class UrbitSSEClient { "event-id": eventId, }; - const { response, release } = await urbitFetch({ - baseUrl: this.url, - path: `/~/channel/${this.channelId}`, - init: { - method: "PUT", - headers: { - "Content-Type": "application/json", - Cookie: this.cookie, - }, - body: JSON.stringify([ackData]), - }, - ssrfPolicy: this.ssrfPolicy, - lookupFn: this.lookupFn, - fetchImpl: this.fetchImpl, + const { response, release } = await this.putChannelPayload([ackData], { timeoutMs: 10_000, auditContext: "tlon-urbit-ack", }); @@ -445,20 +419,7 @@ export class UrbitSSEClient { })); { - const { response, release } = await urbitFetch({ - baseUrl: this.url, - path: `/~/channel/${this.channelId}`, - init: { - method: "PUT", - headers: { - "Content-Type": "application/json", - Cookie: this.cookie, - }, - body: JSON.stringify(unsubscribes), - }, - ssrfPolicy: this.ssrfPolicy, - lookupFn: this.lookupFn, - fetchImpl: this.fetchImpl, + const { response, release } = await this.putChannelPayload(unsubscribes, { timeoutMs: 30_000, auditContext: "tlon-urbit-unsubscribe", }); @@ -501,4 +462,27 @@ export class UrbitSSEClient { await release(); } } + + private async putChannelPayload( + payload: unknown, + params: { timeoutMs: number; auditContext: string }, + ) { + return await urbitFetch({ + baseUrl: this.url, + path: `/~/channel/${this.channelId}`, + init: { + method: "PUT", + headers: { + "Content-Type": "application/json", + Cookie: this.cookie, + }, + body: JSON.stringify(payload), + }, + ssrfPolicy: this.ssrfPolicy, + lookupFn: this.lookupFn, + fetchImpl: this.fetchImpl, + timeoutMs: params.timeoutMs, + auditContext: params.auditContext, + }); + } } diff --git a/extensions/vllm/index.ts b/extensions/vllm/index.ts index fd0a5e18914..3a30f8b9f76 100644 --- a/extensions/vllm/index.ts +++ b/extensions/vllm/index.ts @@ -2,11 +2,9 @@ import { buildVllmProvider, configureOpenAICompatibleSelfHostedProviderNonInteractive, emptyPluginConfigSchema, - promptAndConfigureOpenAICompatibleSelfHostedProvider, + promptAndConfigureOpenAICompatibleSelfHostedProviderAuth, type OpenClawPluginApi, - type ProviderAuthContext, type ProviderAuthMethodNonInteractiveContext, - type ProviderAuthResult, type ProviderDiscoveryContext, } from "openclaw/plugin-sdk/core"; @@ -30,8 +28,8 @@ const vllmPlugin = { label: "vLLM", hint: "Local/self-hosted OpenAI-compatible server", kind: "custom", - run: async (ctx: ProviderAuthContext): Promise => { - const result = await promptAndConfigureOpenAICompatibleSelfHostedProvider({ + run: (ctx) => + promptAndConfigureOpenAICompatibleSelfHostedProviderAuth({ cfg: ctx.config, prompter: ctx.prompter, providerId: PROVIDER_ID, @@ -39,18 +37,7 @@ const vllmPlugin = { defaultBaseUrl: DEFAULT_BASE_URL, defaultApiKeyEnvVar: "VLLM_API_KEY", modelPlaceholder: "meta-llama/Meta-Llama-3-8B-Instruct", - }); - return { - profiles: [ - { - profileId: result.profileId, - credential: result.credential, - }, - ], - configPatch: result.config, - defaultModel: result.modelRef, - }; - }, + }), runNonInteractive: async (ctx: ProviderAuthMethodNonInteractiveContext) => configureOpenAICompatibleSelfHostedProviderNonInteractive({ ctx, diff --git a/extensions/voice-call/index.ts b/extensions/voice-call/index.ts index 8e2fba9898f..e61b5142ef1 100644 --- a/extensions/voice-call/index.ts +++ b/extensions/voice-call/index.ts @@ -227,6 +227,35 @@ const voiceCallPlugin = { params.respond(true, { callId: result.callId, initiated: true }); }; + const respondToCallMessageAction = async (params: { + requestParams: GatewayRequestHandlerOptions["params"]; + respond: GatewayRequestHandlerOptions["respond"]; + action: (request: Awaited>) => Promise<{ + success: boolean; + error?: string; + transcript?: string; + }>; + failure: string; + includeTranscript?: boolean; + }) => { + const request = await resolveCallMessageRequest(params.requestParams); + if ("error" in request) { + params.respond(false, { error: request.error }); + return; + } + const result = await params.action(request); + if (!result.success) { + params.respond(false, { error: result.error || params.failure }); + return; + } + params.respond( + true, + params.includeTranscript + ? { success: true, transcript: result.transcript } + : { success: true }, + ); + }; + api.registerGatewayMethod( "voicecall.initiate", async ({ params, respond }: GatewayRequestHandlerOptions) => { @@ -264,17 +293,13 @@ const voiceCallPlugin = { "voicecall.continue", async ({ params, respond }: GatewayRequestHandlerOptions) => { try { - const request = await resolveCallMessageRequest(params); - if ("error" in request) { - respond(false, { error: request.error }); - return; - } - const result = await request.rt.manager.continueCall(request.callId, request.message); - if (!result.success) { - respond(false, { error: result.error || "continue failed" }); - return; - } - respond(true, { success: true, transcript: result.transcript }); + await respondToCallMessageAction({ + requestParams: params, + respond, + action: (request) => request.rt.manager.continueCall(request.callId, request.message), + failure: "continue failed", + includeTranscript: true, + }); } catch (err) { sendError(respond, err); } @@ -285,17 +310,12 @@ const voiceCallPlugin = { "voicecall.speak", async ({ params, respond }: GatewayRequestHandlerOptions) => { try { - const request = await resolveCallMessageRequest(params); - if ("error" in request) { - respond(false, { error: request.error }); - return; - } - const result = await request.rt.manager.speak(request.callId, request.message); - if (!result.success) { - respond(false, { error: result.error || "speak failed" }); - return; - } - respond(true, { success: true }); + await respondToCallMessageAction({ + requestParams: params, + respond, + action: (request) => request.rt.manager.speak(request.callId, request.message), + failure: "speak failed", + }); } catch (err) { sendError(respond, err); } diff --git a/extensions/zalouser/src/accounts.test-mocks.ts b/extensions/zalouser/src/accounts.test-mocks.ts new file mode 100644 index 00000000000..0206095d0fc --- /dev/null +++ b/extensions/zalouser/src/accounts.test-mocks.ts @@ -0,0 +1,10 @@ +import { vi } from "vitest"; +import { createDefaultResolvedZalouserAccount } from "./test-helpers.js"; + +vi.mock("./accounts.js", async (importOriginal) => { + const actual = (await importOriginal()) as Record; + return { + ...actual, + resolveZalouserAccountSync: () => createDefaultResolvedZalouserAccount(), + }; +}); diff --git a/extensions/zalouser/src/channel.directory.test.ts b/extensions/zalouser/src/channel.directory.test.ts index f8c13b208e4..1736118bc0e 100644 --- a/extensions/zalouser/src/channel.directory.test.ts +++ b/extensions/zalouser/src/channel.directory.test.ts @@ -1,5 +1,6 @@ -import type { RuntimeEnv } from "openclaw/plugin-sdk/zalouser"; import { describe, expect, it, vi } from "vitest"; +import "./accounts.test-mocks.js"; +import { createZalouserRuntimeEnv } from "./test-helpers.js"; const listZaloGroupMembersMock = vi.hoisted(() => vi.fn(async () => [])); @@ -11,30 +12,9 @@ vi.mock("./zalo-js.js", async (importOriginal) => { }; }); -vi.mock("./accounts.js", async (importOriginal) => { - const actual = (await importOriginal()) as Record; - return { - ...actual, - resolveZalouserAccountSync: () => ({ - accountId: "default", - profile: "default", - name: "test", - enabled: true, - authenticated: true, - config: {}, - }), - }; -}); - import { zalouserPlugin } from "./channel.js"; -const runtimeStub: RuntimeEnv = { - log: vi.fn(), - error: vi.fn(), - exit: ((code: number): never => { - throw new Error(`exit ${code}`); - }) as RuntimeEnv["exit"], -}; +const runtimeStub = createZalouserRuntimeEnv(); describe("zalouser directory group members", () => { it("accepts prefixed group ids from directory groups list output", async () => { diff --git a/extensions/zalouser/src/channel.sendpayload.test.ts b/extensions/zalouser/src/channel.sendpayload.test.ts index d388773e2e6..27a8adf2c0d 100644 --- a/extensions/zalouser/src/channel.sendpayload.test.ts +++ b/extensions/zalouser/src/channel.sendpayload.test.ts @@ -1,5 +1,6 @@ import type { ReplyPayload } from "openclaw/plugin-sdk/zalouser"; import { beforeEach, describe, expect, it, vi } from "vitest"; +import "./accounts.test-mocks.js"; import { installSendPayloadContractSuite, primeSendMock, @@ -12,20 +13,6 @@ vi.mock("./send.js", () => ({ sendReactionZalouser: vi.fn().mockResolvedValue({ ok: true }), })); -vi.mock("./accounts.js", async (importOriginal) => { - const actual = (await importOriginal()) as Record; - return { - ...actual, - resolveZalouserAccountSync: () => ({ - accountId: "default", - profile: "default", - name: "test", - enabled: true, - config: {}, - }), - }; -}); - function baseCtx(payload: ReplyPayload) { return { cfg: {}, diff --git a/extensions/zalouser/src/monitor.account-scope.test.ts b/extensions/zalouser/src/monitor.account-scope.test.ts index 919bd25887c..ff8884282ac 100644 --- a/extensions/zalouser/src/monitor.account-scope.test.ts +++ b/extensions/zalouser/src/monitor.account-scope.test.ts @@ -4,6 +4,7 @@ import "./monitor.send-mocks.js"; import { __testing } from "./monitor.js"; import { sendMessageZalouserMock } from "./monitor.send-mocks.js"; import { setZalouserRuntime } from "./runtime.js"; +import { createZalouserRuntimeEnv } from "./test-helpers.js"; import type { ResolvedZalouserAccount, ZaloInboundMessage } from "./types.js"; describe("zalouser monitor pairing account scoping", () => { @@ -80,19 +81,11 @@ describe("zalouser monitor pairing account scoping", () => { raw: { source: "test" }, }; - const runtime: RuntimeEnv = { - log: vi.fn(), - error: vi.fn(), - exit: ((code: number): never => { - throw new Error(`exit ${code}`); - }) as RuntimeEnv["exit"], - }; - await __testing.processMessage({ message, account, config, - runtime, + runtime: createZalouserRuntimeEnv(), }); expect(readAllowFromStore).toHaveBeenCalledWith( diff --git a/extensions/zalouser/src/monitor.group-gating.test.ts b/extensions/zalouser/src/monitor.group-gating.test.ts index ca42edde43a..ef68d6f2529 100644 --- a/extensions/zalouser/src/monitor.group-gating.test.ts +++ b/extensions/zalouser/src/monitor.group-gating.test.ts @@ -9,6 +9,7 @@ import { sendTypingZalouserMock, } from "./monitor.send-mocks.js"; import { setZalouserRuntime } from "./runtime.js"; +import { createZalouserRuntimeEnv } from "./test-helpers.js"; import type { ResolvedZalouserAccount, ZaloInboundMessage } from "./types.js"; function createAccount(): ResolvedZalouserAccount { @@ -39,15 +40,7 @@ function createConfig(): OpenClawConfig { }; } -function createRuntimeEnv(): RuntimeEnv { - return { - log: vi.fn(), - error: vi.fn(), - exit: ((code: number): never => { - throw new Error(`exit ${code}`); - }) as RuntimeEnv["exit"], - }; -} +const createRuntimeEnv = () => createZalouserRuntimeEnv(); function installRuntime(params: { commandAuthorized?: boolean; @@ -269,7 +262,7 @@ describe("zalouser monitor group mention gating", () => { message: params.message, account: params.account ?? createAccount(), config: createConfig(), - runtime: createRuntimeEnv(), + runtime: createZalouserRuntimeEnv(), historyState: params.historyState, }); } diff --git a/extensions/zalouser/src/test-helpers.ts b/extensions/zalouser/src/test-helpers.ts new file mode 100644 index 00000000000..8b43e182c54 --- /dev/null +++ b/extensions/zalouser/src/test-helpers.ts @@ -0,0 +1,26 @@ +import type { RuntimeEnv } from "openclaw/plugin-sdk/zalouser"; +import type { ResolvedZalouserAccount } from "./types.js"; + +export function createZalouserRuntimeEnv(): RuntimeEnv { + return { + log: () => {}, + error: () => {}, + exit: ((code: number): never => { + throw new Error(`exit ${code}`); + }) as RuntimeEnv["exit"], + }; +} + +export function createDefaultResolvedZalouserAccount( + overrides: Partial = {}, +): ResolvedZalouserAccount { + return { + accountId: "default", + profile: "default", + name: "test", + enabled: true, + authenticated: true, + config: {}, + ...overrides, + }; +} diff --git a/scripts/test-parallel.mjs b/scripts/test-parallel.mjs index 1716f724bff..021ff1f905e 100644 --- a/scripts/test-parallel.mjs +++ b/scripts/test-parallel.mjs @@ -1,6 +1,7 @@ import { spawn } from "node:child_process"; import fs from "node:fs"; import os from "node:os"; +import path from "node:path"; // On Windows, `.cmd` launchers can fail with `spawn EINVAL` when invoked without a shell // (especially under GitHub Actions + Git Bash). Use `shell: true` and let the shell resolve pnpm. @@ -205,6 +206,45 @@ const shardIndexOverride = (() => { const parsed = Number.parseInt(process.env.OPENCLAW_TEST_SHARD_INDEX ?? "", 10); return Number.isFinite(parsed) && parsed > 0 ? parsed : null; })(); +const OPTION_TAKES_VALUE = new Set([ + "-t", + "-c", + "-r", + "--testNamePattern", + "--config", + "--root", + "--dir", + "--reporter", + "--outputFile", + "--pool", + "--execArgv", + "--vmMemoryLimit", + "--maxWorkers", + "--environment", + "--shard", + "--changed", + "--sequence", + "--inspect", + "--inspectBrk", + "--testTimeout", + "--hookTimeout", + "--bail", + "--retry", + "--diff", + "--exclude", + "--project", + "--slowTestThreshold", + "--teardownTimeout", + "--attachmentsDir", + "--mode", + "--api", + "--browser", + "--maxConcurrency", + "--mergeReports", + "--configLoader", + "--experimental", +]); +const SINGLE_RUN_ONLY_FLAGS = new Set(["--coverage", "--outputFile", "--mergeReports"]); if (shardIndexOverride !== null && shardCount <= 1) { console.error( @@ -229,6 +269,219 @@ const silentArgs = const rawPassthroughArgs = process.argv.slice(2); const passthroughArgs = rawPassthroughArgs[0] === "--" ? rawPassthroughArgs.slice(1) : rawPassthroughArgs; +const parsePassthroughArgs = (args) => { + const fileFilters = []; + const optionArgs = []; + let consumeNextAsOptionValue = false; + + for (const arg of args) { + if (consumeNextAsOptionValue) { + optionArgs.push(arg); + consumeNextAsOptionValue = false; + continue; + } + if (arg === "--") { + optionArgs.push(arg); + continue; + } + if (arg.startsWith("-")) { + optionArgs.push(arg); + consumeNextAsOptionValue = !arg.includes("=") && OPTION_TAKES_VALUE.has(arg); + continue; + } + fileFilters.push(arg); + } + + return { fileFilters, optionArgs }; +}; +const { fileFilters: passthroughFileFilters, optionArgs: passthroughOptionArgs } = + parsePassthroughArgs(passthroughArgs); +const passthroughRequiresSingleRun = passthroughOptionArgs.some((arg) => { + if (!arg.startsWith("-")) { + return false; + } + const [flag] = arg.split("=", 1); + return SINGLE_RUN_ONLY_FLAGS.has(flag); +}); +const channelPrefixes = ["src/telegram/", "src/discord/", "src/web/", "src/browser/", "src/line/"]; +const baseConfigPrefixes = ["src/agents/", "src/auto-reply/", "src/commands/", "test/", "ui/"]; +const normalizeRepoPath = (value) => value.split(path.sep).join("/"); +const walkTestFiles = (rootDir) => { + if (!fs.existsSync(rootDir)) { + return []; + } + const entries = fs.readdirSync(rootDir, { withFileTypes: true }); + const files = []; + for (const entry of entries) { + const fullPath = path.join(rootDir, entry.name); + if (entry.isDirectory()) { + files.push(...walkTestFiles(fullPath)); + continue; + } + if (!entry.isFile()) { + continue; + } + if ( + fullPath.endsWith(".test.ts") || + fullPath.endsWith(".live.test.ts") || + fullPath.endsWith(".e2e.test.ts") + ) { + files.push(normalizeRepoPath(fullPath)); + } + } + return files; +}; +const allKnownTestFiles = [ + ...new Set([ + ...walkTestFiles("src"), + ...walkTestFiles("extensions"), + ...walkTestFiles("test"), + ...walkTestFiles(path.join("ui", "src", "ui")), + ]), +]; +const inferTarget = (fileFilter) => { + const isolated = unitIsolatedFiles.includes(fileFilter); + if (fileFilter.endsWith(".live.test.ts")) { + return { owner: "live", isolated }; + } + if (fileFilter.endsWith(".e2e.test.ts")) { + return { owner: "e2e", isolated }; + } + if (fileFilter.startsWith("extensions/")) { + return { owner: "extensions", isolated }; + } + if (fileFilter.startsWith("src/gateway/")) { + return { owner: "gateway", isolated }; + } + if (channelPrefixes.some((prefix) => fileFilter.startsWith(prefix))) { + return { owner: "channels", isolated }; + } + if (baseConfigPrefixes.some((prefix) => fileFilter.startsWith(prefix))) { + return { owner: "base", isolated }; + } + if (fileFilter.startsWith("src/")) { + return { owner: "unit", isolated }; + } + return { owner: "base", isolated }; +}; +const resolveFilterMatches = (fileFilter) => { + const normalizedFilter = normalizeRepoPath(fileFilter); + if (fs.existsSync(fileFilter)) { + const stats = fs.statSync(fileFilter); + if (stats.isFile()) { + return [normalizedFilter]; + } + if (stats.isDirectory()) { + const prefix = normalizedFilter.endsWith("/") ? normalizedFilter : `${normalizedFilter}/`; + return allKnownTestFiles.filter((file) => file.startsWith(prefix)); + } + } + if (/[*?[\]{}]/.test(normalizedFilter)) { + return allKnownTestFiles.filter((file) => path.matchesGlob(file, normalizedFilter)); + } + return allKnownTestFiles.filter((file) => file.includes(normalizedFilter)); +}; +const createTargetedEntry = (owner, isolated, filters) => { + const name = isolated ? `${owner}-isolated` : owner; + const forceForks = isolated; + if (owner === "unit") { + return { + name, + args: [ + "vitest", + "run", + "--config", + "vitest.unit.config.ts", + `--pool=${forceForks ? "forks" : useVmForks ? "vmForks" : "forks"}`, + ...(disableIsolation ? ["--isolate=false"] : []), + ...filters, + ], + }; + } + if (owner === "extensions") { + return { + name, + args: [ + "vitest", + "run", + "--config", + "vitest.extensions.config.ts", + ...(forceForks ? ["--pool=forks"] : useVmForks ? ["--pool=vmForks"] : []), + ...filters, + ], + }; + } + if (owner === "gateway") { + return { + name, + args: ["vitest", "run", "--config", "vitest.gateway.config.ts", "--pool=forks", ...filters], + }; + } + if (owner === "channels") { + return { + name, + args: [ + "vitest", + "run", + "--config", + "vitest.channels.config.ts", + ...(forceForks ? ["--pool=forks"] : []), + ...filters, + ], + }; + } + if (owner === "live") { + return { + name, + args: ["vitest", "run", "--config", "vitest.live.config.ts", ...filters], + }; + } + if (owner === "e2e") { + return { + name, + args: ["vitest", "run", "--config", "vitest.e2e.config.ts", ...filters], + }; + } + return { + name, + args: [ + "vitest", + "run", + "--config", + "vitest.config.ts", + ...(forceForks ? ["--pool=forks"] : []), + ...filters, + ], + }; +}; +const targetedEntries = (() => { + if (passthroughFileFilters.length === 0) { + return []; + } + const groups = passthroughFileFilters.reduce((acc, fileFilter) => { + const matchedFiles = resolveFilterMatches(fileFilter); + if (matchedFiles.length === 0) { + const target = inferTarget(normalizeRepoPath(fileFilter)); + const key = `${target.owner}:${target.isolated ? "isolated" : "default"}`; + const files = acc.get(key) ?? []; + files.push(normalizeRepoPath(fileFilter)); + acc.set(key, files); + return acc; + } + for (const matchedFile of matchedFiles) { + const target = inferTarget(matchedFile); + const key = `${target.owner}:${target.isolated ? "isolated" : "default"}`; + const files = acc.get(key) ?? []; + files.push(matchedFile); + acc.set(key, files); + } + return acc; + }, new Map()); + return Array.from(groups, ([key, filters]) => { + const [owner, mode] = key.split(":"); + return createTargetedEntry(owner, mode === "isolated", [...new Set(filters)]); + }); +})(); const topLevelParallelEnabled = testProfile !== "low" && testProfile !== "serial"; const overrideWorkers = Number.parseInt(process.env.OPENCLAW_TEST_WORKERS ?? "", 10); const resolvedOverride = @@ -311,7 +564,7 @@ const maxWorkersForRun = (name) => { if (isCI && isMacOS) { return 1; } - if (name === "unit-isolated") { + if (name === "unit-isolated" || name.endsWith("-isolated")) { return defaultWorkerBudget.unitIsolated; } if (name === "extensions") { @@ -397,16 +650,16 @@ const runOnce = (entry, extraArgs = []) => }); }); -const run = async (entry) => { +const run = async (entry, extraArgs = []) => { if (shardCount <= 1) { - return runOnce(entry); + return runOnce(entry, extraArgs); } if (shardIndexOverride !== null) { - return runOnce(entry, ["--shard", `${shardIndexOverride}/${shardCount}`]); + return runOnce(entry, ["--shard", `${shardIndexOverride}/${shardCount}`, ...extraArgs]); } for (let shardIndex = 1; shardIndex <= shardCount; shardIndex += 1) { // eslint-disable-next-line no-await-in-loop - const code = await runOnce(entry, ["--shard", `${shardIndex}/${shardCount}`]); + const code = await runOnce(entry, ["--shard", `${shardIndex}/${shardCount}`, ...extraArgs]); if (code !== 0) { return code; } @@ -414,15 +667,15 @@ const run = async (entry) => { return 0; }; -const runEntries = async (entries) => { +const runEntries = async (entries, extraArgs = []) => { if (topLevelParallelEnabled) { - const codes = await Promise.all(entries.map(run)); + const codes = await Promise.all(entries.map((entry) => run(entry, extraArgs))); return codes.find((code) => code !== 0); } for (const entry of entries) { // eslint-disable-next-line no-await-in-loop - const code = await run(entry); + const code = await run(entry, extraArgs); if (code !== 0) { return code; } @@ -440,57 +693,48 @@ const shutdown = (signal) => { process.on("SIGINT", () => shutdown("SIGINT")); process.on("SIGTERM", () => shutdown("SIGTERM")); -if (passthroughArgs.length > 0) { - const maxWorkers = maxWorkersForRun("unit"); - const args = maxWorkers - ? [ - "vitest", - "run", - "--maxWorkers", - String(maxWorkers), - ...silentArgs, - ...windowsCiArgs, - ...passthroughArgs, - ] - : ["vitest", "run", ...silentArgs, ...windowsCiArgs, ...passthroughArgs]; - const nodeOptions = process.env.NODE_OPTIONS ?? ""; - const nextNodeOptions = WARNING_SUPPRESSION_FLAGS.reduce( - (acc, flag) => (acc.includes(flag) ? acc : `${acc} ${flag}`.trim()), - nodeOptions, - ); - const code = await new Promise((resolve) => { - let child; - try { - child = spawn(pnpm, args, { - stdio: "inherit", - env: { ...process.env, NODE_OPTIONS: nextNodeOptions }, - shell: isWindows, - }); - } catch (err) { - console.error(`[test-parallel] spawn failed: ${String(err)}`); - resolve(1); - return; +if (targetedEntries.length > 0) { + if (passthroughRequiresSingleRun && targetedEntries.length > 1) { + console.error( + "[test-parallel] The provided Vitest args require a single run, but the selected test filters span multiple wrapper configs. Run one target/config at a time.", + ); + process.exit(2); + } + const targetedParallelRuns = keepGatewaySerial + ? targetedEntries.filter((entry) => entry.name !== "gateway") + : targetedEntries; + const targetedSerialRuns = keepGatewaySerial + ? targetedEntries.filter((entry) => entry.name === "gateway") + : []; + const failedTargetedParallel = await runEntries(targetedParallelRuns, passthroughOptionArgs); + if (failedTargetedParallel !== undefined) { + process.exit(failedTargetedParallel); + } + for (const entry of targetedSerialRuns) { + // eslint-disable-next-line no-await-in-loop + const code = await run(entry, passthroughOptionArgs); + if (code !== 0) { + process.exit(code); } - children.add(child); - child.on("error", (err) => { - console.error(`[test-parallel] child error: ${String(err)}`); - }); - child.on("exit", (exitCode, signal) => { - children.delete(child); - resolve(exitCode ?? (signal ? 1 : 0)); - }); - }); - process.exit(Number(code) || 0); + } + process.exit(0); } -const failedParallel = await runEntries(parallelRuns); +if (passthroughRequiresSingleRun && passthroughOptionArgs.length > 0) { + console.error( + "[test-parallel] The provided Vitest args require a single run. Use the dedicated npm script for that workflow (for example `pnpm test:coverage`) or target a single test file/filter.", + ); + process.exit(2); +} + +const failedParallel = await runEntries(parallelRuns, passthroughOptionArgs); if (failedParallel !== undefined) { process.exit(failedParallel); } for (const entry of serialRuns) { // eslint-disable-next-line no-await-in-loop - const code = await run(entry); + const code = await run(entry, passthroughOptionArgs); if (code !== 0) { process.exit(code); } diff --git a/src/agents/pi-embedded-runner/anthropic-stream-wrappers.ts b/src/agents/pi-embedded-runner/anthropic-stream-wrappers.ts index 19b5701eaaa..e04de8a5d6b 100644 --- a/src/agents/pi-embedded-runner/anthropic-stream-wrappers.ts +++ b/src/agents/pi-embedded-runner/anthropic-stream-wrappers.ts @@ -75,6 +75,17 @@ function resolveAnthropicFastServiceTier(enabled: boolean): AnthropicServiceTier return enabled ? "auto" : "standard_only"; } +function hasOpenAiAnthropicToolPayloadCompatFlag(model: { compat?: unknown }): boolean { + if (!model.compat || typeof model.compat !== "object" || Array.isArray(model.compat)) { + return false; + } + + return ( + (model.compat as { requiresOpenAiAnthropicToolPayload?: unknown }) + .requiresOpenAiAnthropicToolPayload === true + ); +} + function requiresAnthropicToolPayloadCompatibilityForModel(model: { api?: unknown; provider?: unknown; @@ -90,15 +101,7 @@ function requiresAnthropicToolPayloadCompatibilityForModel(model: { ) { return true; } - - if (!model.compat || typeof model.compat !== "object" || Array.isArray(model.compat)) { - return false; - } - - return ( - (model.compat as { requiresOpenAiAnthropicToolPayload?: unknown }) - .requiresOpenAiAnthropicToolPayload === true - ); + return hasOpenAiAnthropicToolPayloadCompatFlag(model); } function usesOpenAiFunctionAnthropicToolSchemaForModel(model: { @@ -108,13 +111,7 @@ function usesOpenAiFunctionAnthropicToolSchemaForModel(model: { if (typeof model.provider === "string" && usesOpenAiFunctionAnthropicToolSchema(model.provider)) { return true; } - if (!model.compat || typeof model.compat !== "object" || Array.isArray(model.compat)) { - return false; - } - return ( - (model.compat as { requiresOpenAiAnthropicToolPayload?: unknown }) - .requiresOpenAiAnthropicToolPayload === true - ); + return hasOpenAiAnthropicToolPayloadCompatFlag(model); } function usesOpenAiStringModeAnthropicToolChoiceForModel(model: { @@ -127,13 +124,7 @@ function usesOpenAiStringModeAnthropicToolChoiceForModel(model: { ) { return true; } - if (!model.compat || typeof model.compat !== "object" || Array.isArray(model.compat)) { - return false; - } - return ( - (model.compat as { requiresOpenAiAnthropicToolPayload?: unknown }) - .requiresOpenAiAnthropicToolPayload === true - ); + return hasOpenAiAnthropicToolPayloadCompatFlag(model); } function normalizeOpenAiFunctionAnthropicToolDefinition( diff --git a/src/auto-reply/reply.stage-sandbox-media.scp-remote-path.test.ts b/src/auto-reply/reply.stage-sandbox-media.scp-remote-path.test.ts new file mode 100644 index 00000000000..d5d628421d9 --- /dev/null +++ b/src/auto-reply/reply.stage-sandbox-media.scp-remote-path.test.ts @@ -0,0 +1,75 @@ +import fs from "node:fs/promises"; +import { basename, join } from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { + createSandboxMediaContexts, + createSandboxMediaStageConfig, + withSandboxMediaTempHome, +} from "./stage-sandbox-media.test-harness.js"; + +const sandboxMocks = vi.hoisted(() => ({ + ensureSandboxWorkspaceForSession: vi.fn(), +})); +const childProcessMocks = vi.hoisted(() => ({ + spawn: vi.fn(), +})); + +vi.mock("../agents/sandbox.js", () => sandboxMocks); +vi.mock("node:child_process", () => childProcessMocks); + +import { stageSandboxMedia } from "./reply/stage-sandbox-media.js"; + +afterEach(() => { + vi.restoreAllMocks(); + childProcessMocks.spawn.mockClear(); +}); + +function createRemoteStageParams(home: string): { + cfg: ReturnType; + workspaceDir: string; + sessionKey: string; + remoteCacheDir: string; +} { + const sessionKey = "agent:main:main"; + vi.mocked(sandboxMocks.ensureSandboxWorkspaceForSession).mockResolvedValue(null); + return { + cfg: createSandboxMediaStageConfig(home), + workspaceDir: join(home, "openclaw"), + sessionKey, + remoteCacheDir: join(home, ".openclaw", "media", "remote-cache", sessionKey), + }; +} + +function createRemoteContexts(remotePath: string) { + const { ctx, sessionCtx } = createSandboxMediaContexts(remotePath); + ctx.Provider = "imessage"; + ctx.MediaRemoteHost = "user@gateway-host"; + sessionCtx.Provider = "imessage"; + sessionCtx.MediaRemoteHost = "user@gateway-host"; + return { ctx, sessionCtx }; +} + +describe("stageSandboxMedia scp remote paths", () => { + it("rejects remote attachment filenames with shell metacharacters before spawning scp", async () => { + await withSandboxMediaTempHome("openclaw-triggers-", async (home) => { + const { cfg, workspaceDir, sessionKey, remoteCacheDir } = createRemoteStageParams(home); + const remotePath = "/Users/demo/Library/Messages/Attachments/ab/cd/evil$(touch pwned).jpg"; + const { ctx, sessionCtx } = createRemoteContexts(remotePath); + + await stageSandboxMedia({ + ctx, + sessionCtx, + cfg, + sessionKey, + workspaceDir, + }); + + expect(childProcessMocks.spawn).not.toHaveBeenCalled(); + await expect(fs.stat(join(remoteCacheDir, basename(remotePath)))).rejects.toThrow(); + expect(ctx.MediaPath).toBe(remotePath); + expect(sessionCtx.MediaPath).toBe(remotePath); + expect(ctx.MediaUrl).toBe(remotePath); + expect(sessionCtx.MediaUrl).toBe(remotePath); + }); + }); +}); diff --git a/src/auto-reply/reply/agent-runner-execution.ts b/src/auto-reply/reply/agent-runner-execution.ts index ff3838a1936..27a31c2387a 100644 --- a/src/auto-reply/reply/agent-runner-execution.ts +++ b/src/auto-reply/reply/agent-runner-execution.ts @@ -40,8 +40,7 @@ import { } from "../tokens.js"; import type { GetReplyOptions, ReplyPayload } from "../types.js"; import { - buildEmbeddedRunBaseParams, - buildEmbeddedRunContexts, + buildEmbeddedRunExecutionParams, resolveModelFallbackOptions, } from "./agent-runner-utils.js"; import { type BlockReplyPipeline } from "./block-reply-pipeline.js"; @@ -308,20 +307,17 @@ export async function runAgentTurnWithFallback(params: { } })(); } - const { authProfile, embeddedContext, senderContext } = buildEmbeddedRunContexts({ - run: params.followupRun.run, - sessionCtx: params.sessionCtx, - hasRepliedRef: params.opts?.hasRepliedRef, - provider, - }); - const runBaseParams = buildEmbeddedRunBaseParams({ - run: params.followupRun.run, - provider, - model, - runId, - authProfile, - allowTransientCooldownProbe: runOptions?.allowTransientCooldownProbe, - }); + const { embeddedContext, senderContext, runBaseParams } = buildEmbeddedRunExecutionParams( + { + run: params.followupRun.run, + sessionCtx: params.sessionCtx, + hasRepliedRef: params.opts?.hasRepliedRef, + provider, + runId, + allowTransientCooldownProbe: runOptions?.allowTransientCooldownProbe, + model, + }, + ); return (async () => { const result = await runEmbeddedPiAgent({ ...embeddedContext, diff --git a/src/auto-reply/reply/agent-runner-memory.ts b/src/auto-reply/reply/agent-runner-memory.ts index 623bb9c1490..d52c6d05761 100644 --- a/src/auto-reply/reply/agent-runner-memory.ts +++ b/src/auto-reply/reply/agent-runner-memory.ts @@ -27,8 +27,7 @@ import type { TemplateContext } from "../templating.js"; import type { VerboseLevel } from "../thinking.js"; import type { GetReplyOptions } from "../types.js"; import { - buildEmbeddedRunBaseParams, - buildEmbeddedRunContexts, + buildEmbeddedRunExecutionParams, resolveModelFallbackOptions, } from "./agent-runner-utils.js"; import { @@ -482,18 +481,13 @@ export async function runMemoryFlushIfNeeded(params: { ...resolveModelFallbackOptions(params.followupRun.run), runId: flushRunId, run: async (provider, model, runOptions) => { - const { authProfile, embeddedContext, senderContext } = buildEmbeddedRunContexts({ + const { embeddedContext, senderContext, runBaseParams } = buildEmbeddedRunExecutionParams({ run: params.followupRun.run, sessionCtx: params.sessionCtx, hasRepliedRef: params.opts?.hasRepliedRef, provider, - }); - const runBaseParams = buildEmbeddedRunBaseParams({ - run: params.followupRun.run, - provider, model, runId: flushRunId, - authProfile, allowTransientCooldownProbe: runOptions?.allowTransientCooldownProbe, }); const result = await runEmbeddedPiAgent({ diff --git a/src/auto-reply/reply/agent-runner-payloads.test.ts b/src/auto-reply/reply/agent-runner-payloads.test.ts index 26f23d7a42c..db237848e3c 100644 --- a/src/auto-reply/reply/agent-runner-payloads.test.ts +++ b/src/auto-reply/reply/agent-runner-payloads.test.ts @@ -9,6 +9,20 @@ const baseParams = { replyToMode: "off" as const, }; +async function expectSameTargetRepliesSuppressed(params: { provider: string; to: string }) { + const { replyPayloads } = await buildReplyPayloads({ + ...baseParams, + payloads: [{ text: "hello world!" }], + messageProvider: "heartbeat", + originatingChannel: "feishu", + originatingTo: "ou_abc123", + messagingToolSentTexts: ["different message"], + messagingToolSentTargets: [{ tool: "message", provider: params.provider, to: params.to }], + }); + + expect(replyPayloads).toHaveLength(0); +} + describe("buildReplyPayloads media filter integration", () => { it("strips media URL from payload when in messagingToolSentMediaUrls", async () => { const { replyPayloads } = await buildReplyPayloads({ @@ -142,31 +156,11 @@ describe("buildReplyPayloads media filter integration", () => { }); it("suppresses same-target replies when message tool target provider is generic", async () => { - const { replyPayloads } = await buildReplyPayloads({ - ...baseParams, - payloads: [{ text: "hello world!" }], - messageProvider: "heartbeat", - originatingChannel: "feishu", - originatingTo: "ou_abc123", - messagingToolSentTexts: ["different message"], - messagingToolSentTargets: [{ tool: "message", provider: "message", to: "ou_abc123" }], - }); - - expect(replyPayloads).toHaveLength(0); + await expectSameTargetRepliesSuppressed({ provider: "message", to: "ou_abc123" }); }); it("suppresses same-target replies when target provider is channel alias", async () => { - const { replyPayloads } = await buildReplyPayloads({ - ...baseParams, - payloads: [{ text: "hello world!" }], - messageProvider: "heartbeat", - originatingChannel: "feishu", - originatingTo: "ou_abc123", - messagingToolSentTexts: ["different message"], - messagingToolSentTargets: [{ tool: "message", provider: "lark", to: "ou_abc123" }], - }); - - expect(replyPayloads).toHaveLength(0); + await expectSameTargetRepliesSuppressed({ provider: "lark", to: "ou_abc123" }); }); it("drops all final payloads when block pipeline streamed successfully", async () => { diff --git a/src/auto-reply/reply/agent-runner-utils.ts b/src/auto-reply/reply/agent-runner-utils.ts index 99b2b6392f6..c6e71a9bab0 100644 --- a/src/auto-reply/reply/agent-runner-utils.ts +++ b/src/auto-reply/reply/agent-runner-utils.ts @@ -263,6 +263,31 @@ export function buildEmbeddedRunContexts(params: { }; } +export function buildEmbeddedRunExecutionParams(params: { + run: FollowupRun["run"]; + sessionCtx: TemplateContext; + hasRepliedRef: { value: boolean } | undefined; + provider: string; + model: string; + runId: string; + allowTransientCooldownProbe?: boolean; +}) { + const { authProfile, embeddedContext, senderContext } = buildEmbeddedRunContexts(params); + const runBaseParams = buildEmbeddedRunBaseParams({ + run: params.run, + provider: params.provider, + model: params.model, + runId: params.runId, + authProfile, + allowTransientCooldownProbe: params.allowTransientCooldownProbe, + }); + return { + embeddedContext, + senderContext, + runBaseParams, + }; +} + export function resolveProviderScopedAuthProfile(params: { provider: string; primaryProvider: string; diff --git a/src/auto-reply/reply/agent-runner.media-paths.test.ts b/src/auto-reply/reply/agent-runner.media-paths.test.ts index f5658287aff..a759c539bdc 100644 --- a/src/auto-reply/reply/agent-runner.media-paths.test.ts +++ b/src/auto-reply/reply/agent-runner.media-paths.test.ts @@ -2,7 +2,7 @@ import path from "node:path"; import { beforeEach, describe, expect, it, vi } from "vitest"; import type { TemplateContext } from "../templating.js"; import type { FollowupRun, QueueSettings } from "./queue.js"; -import { createMockTypingController } from "./test-helpers.js"; +import { createMockFollowupRun, createMockTypingController } from "./test-helpers.js"; const runEmbeddedPiAgentMock = vi.fn(); const runWithModelFallbackMock = vi.fn(); @@ -72,32 +72,15 @@ describe("runReplyAgent media path normalization", () => { const result = await runReplyAgent({ commandBody: "generate", - followupRun: { + followupRun: createMockFollowupRun({ prompt: "generate", - enqueuedAt: Date.now(), run: { agentId: "main", agentDir: "/tmp/agent", - sessionId: "session", - sessionKey: "main", messageProvider: "telegram", - sessionFile: "/tmp/session.jsonl", workspaceDir: "/tmp/workspace", - config: {}, - provider: "anthropic", - model: "claude", - thinkLevel: "low", - verboseLevel: "off", - elevatedLevel: "off", - bashElevated: { - enabled: false, - allowed: false, - defaultLevel: "off", - }, - timeoutMs: 1_000, - blockReplyBreak: "message_end", }, - } as unknown as FollowupRun, + }) as unknown as FollowupRun, queueKey: "main", resolvedQueue: { mode: "interrupt" } as QueueSettings, shouldSteer: false, diff --git a/src/auto-reply/reply/block-streaming.ts b/src/auto-reply/reply/block-streaming.ts index 6d306b166c1..b24ee8cac1a 100644 --- a/src/auto-reply/reply/block-streaming.ts +++ b/src/auto-reply/reply/block-streaming.ts @@ -26,6 +26,22 @@ function normalizeChunkProvider(provider?: string): TextChunkProvider | undefine : undefined; } +function resolveProviderChunkContext( + cfg: OpenClawConfig | undefined, + provider?: string, + accountId?: string | null, +) { + const providerKey = normalizeChunkProvider(provider); + const providerId = providerKey ? normalizeChannelId(providerKey) : null; + const providerChunkLimit = providerId + ? getChannelDock(providerId)?.outbound?.textChunkLimit + : undefined; + const textLimit = resolveTextChunkLimit(cfg, providerKey, accountId, { + fallbackLimit: providerChunkLimit, + }); + return { providerKey, providerId, textLimit }; +} + type ProviderBlockStreamingConfig = { blockStreamingCoalesce?: BlockStreamingCoalesceConfig; accounts?: Record; @@ -97,14 +113,7 @@ export function resolveEffectiveBlockStreamingConfig(params: { chunking: BlockStreamingChunking; coalescing: BlockStreamingCoalescing; } { - const providerKey = normalizeChunkProvider(params.provider); - const providerId = providerKey ? normalizeChannelId(providerKey) : null; - const providerChunkLimit = providerId - ? getChannelDock(providerId)?.outbound?.textChunkLimit - : undefined; - const textLimit = resolveTextChunkLimit(params.cfg, providerKey, params.accountId, { - fallbackLimit: providerChunkLimit, - }); + const { textLimit } = resolveProviderChunkContext(params.cfg, params.provider, params.accountId); const chunkingDefaults = params.chunking ?? resolveBlockStreamingChunking(params.cfg, params.provider, params.accountId); const chunkingMax = clampPositiveInteger(params.maxChunkChars, chunkingDefaults.maxChars, { @@ -154,21 +163,13 @@ export function resolveBlockStreamingChunking( provider?: string, accountId?: string | null, ): BlockStreamingChunking { - const providerKey = normalizeChunkProvider(provider); - const providerConfigKey = providerKey; - const providerId = providerKey ? normalizeChannelId(providerKey) : null; - const providerChunkLimit = providerId - ? getChannelDock(providerId)?.outbound?.textChunkLimit - : undefined; - const textLimit = resolveTextChunkLimit(cfg, providerConfigKey, accountId, { - fallbackLimit: providerChunkLimit, - }); + const { providerKey, textLimit } = resolveProviderChunkContext(cfg, provider, accountId); const chunkCfg = cfg?.agents?.defaults?.blockStreamingChunk; // When chunkMode="newline", the outbound delivery splits on paragraph boundaries. // The block chunker should flush eagerly on \n\n boundaries during streaming, // regardless of minChars, so each paragraph is sent as its own message. - const chunkMode = resolveChunkMode(cfg, providerConfigKey, accountId); + const chunkMode = resolveChunkMode(cfg, providerKey, accountId); const maxRequested = Math.max(1, Math.floor(chunkCfg?.maxChars ?? DEFAULT_BLOCK_STREAM_MAX)); const maxChars = Math.max(1, Math.min(maxRequested, textLimit)); @@ -198,20 +199,15 @@ export function resolveBlockStreamingCoalescing( }, opts?: { chunkMode?: "length" | "newline" }, ): BlockStreamingCoalescing | undefined { - const providerKey = normalizeChunkProvider(provider); - const providerConfigKey = providerKey; + const { providerKey, providerId, textLimit } = resolveProviderChunkContext( + cfg, + provider, + accountId, + ); // Resolve the outbound chunkMode so the coalescer can flush on paragraph boundaries // when chunkMode="newline", matching the delivery-time splitting behavior. - const chunkMode = opts?.chunkMode ?? resolveChunkMode(cfg, providerConfigKey, accountId); - - const providerId = providerKey ? normalizeChannelId(providerKey) : null; - const providerChunkLimit = providerId - ? getChannelDock(providerId)?.outbound?.textChunkLimit - : undefined; - const textLimit = resolveTextChunkLimit(cfg, providerConfigKey, accountId, { - fallbackLimit: providerChunkLimit, - }); + const chunkMode = opts?.chunkMode ?? resolveChunkMode(cfg, providerKey, accountId); const providerDefaults = providerId ? getChannelDock(providerId)?.streaming?.blockStreamingCoalesceDefaults : undefined; diff --git a/src/auto-reply/reply/commands-acp/context.ts b/src/auto-reply/reply/commands-acp/context.ts index fd90175f38a..84acb828015 100644 --- a/src/auto-reply/reply/commands-acp/context.ts +++ b/src/auto-reply/reply/commands-acp/context.ts @@ -5,8 +5,8 @@ import { } from "../../../acp/conversation-id.js"; import { DISCORD_THREAD_BINDING_CHANNEL } from "../../../channels/thread-bindings-policy.js"; import { resolveConversationIdFromTargets } from "../../../infra/outbound/conversation-id.js"; -import { parseAgentSessionKey } from "../../../routing/session-key.js"; import type { HandleCommandsParams } from "../commands-types.js"; +import { parseDiscordParentChannelFromSessionKey } from "../discord-parent-channel.js"; import { resolveTelegramConversationId } from "../telegram-context.js"; export function resolveAcpCommandChannel(params: HandleCommandsParams): string { @@ -64,19 +64,6 @@ export function resolveAcpCommandConversationId(params: HandleCommandsParams): s }); } -function parseDiscordParentChannelFromSessionKey(raw: unknown): string | undefined { - const sessionKey = normalizeConversationText(raw); - if (!sessionKey) { - return undefined; - } - const scoped = parseAgentSessionKey(sessionKey)?.rest ?? sessionKey.toLowerCase(); - const match = scoped.match(/(?:^|:)channel:([^:]+)$/); - if (!match?.[1]) { - return undefined; - } - return match[1]; -} - function parseDiscordParentChannelFromContext(raw: unknown): string | undefined { const parentId = normalizeConversationText(raw); if (!parentId) { diff --git a/src/auto-reply/reply/commands-allowlist.ts b/src/auto-reply/reply/commands-allowlist.ts index ffba3bf2505..fcecb0b31f3 100644 --- a/src/auto-reply/reply/commands-allowlist.ts +++ b/src/auto-reply/reply/commands-allowlist.ts @@ -1,10 +1,5 @@ import { getChannelDock } from "../../channels/dock.js"; -import { - authorizeConfigWrite, - canBypassConfigWritePolicy, - formatConfigWriteDeniedMessage, - resolveExplicitConfigWriteTarget, -} from "../../channels/plugins/config-writes.js"; +import { resolveExplicitConfigWriteTarget } from "../../channels/plugins/config-writes.js"; import { listPairingChannels } from "../../channels/plugins/pairing.js"; import type { ChannelId } from "../../channels/plugins/types.js"; import { normalizeChannelId } from "../../channels/registry.js"; @@ -36,6 +31,7 @@ import { resolveTelegramAccount } from "../../telegram/accounts.js"; import { resolveWhatsAppAccount } from "../../web/accounts.js"; import { rejectUnauthorizedCommand, requireCommandFlagEnabled } from "./command-gates.js"; import type { CommandHandler } from "./commands-types.js"; +import { resolveConfigWriteDeniedText } from "./config-write-authorization.js"; type AllowlistScope = "dm" | "group" | "all"; type AllowlistAction = "list" | "add" | "remove"; @@ -628,20 +624,19 @@ export const handleAllowlistCommand: CommandHandler = async (params, allowTextCo accountId: normalizedAccountId, writeTarget, } = resolveAccountTarget(parsedConfig, channelId, accountId); - const writeAuth = authorizeConfigWrite({ + const deniedText = resolveConfigWriteDeniedText({ cfg: params.cfg, - origin: { channelId, accountId: params.ctx.AccountId }, + channel: params.command.channel, + channelId, + accountId: params.ctx.AccountId, + gatewayClientScopes: params.ctx.GatewayClientScopes, target: writeTarget, - allowBypass: canBypassConfigWritePolicy({ - channel: params.command.channel, - gatewayClientScopes: params.ctx.GatewayClientScopes, - }), }); - if (!writeAuth.allowed) { + if (deniedText) { return { shouldContinue: false, reply: { - text: formatConfigWriteDeniedMessage({ result: writeAuth, fallbackChannelId: channelId }), + text: deniedText, }, }; } diff --git a/src/auto-reply/reply/commands-config.ts b/src/auto-reply/reply/commands-config.ts index 96b5a5d9be5..b40032758d3 100644 --- a/src/auto-reply/reply/commands-config.ts +++ b/src/auto-reply/reply/commands-config.ts @@ -1,9 +1,4 @@ -import { - authorizeConfigWrite, - canBypassConfigWritePolicy, - formatConfigWriteDeniedMessage, - resolveConfigWriteTargetFromPath, -} from "../../channels/plugins/config-writes.js"; +import { resolveConfigWriteTargetFromPath } from "../../channels/plugins/config-writes.js"; import { normalizeChannelId } from "../../channels/registry.js"; import { getConfigValueAtPath, @@ -31,6 +26,7 @@ import { } from "./command-gates.js"; import type { CommandHandler } from "./commands-types.js"; import { parseConfigCommand } from "./config-commands.js"; +import { resolveConfigWriteDeniedText } from "./config-write-authorization.js"; import { parseDebugCommand } from "./debug-commands.js"; export const handleConfigCommand: CommandHandler = async (params, allowTextCommands) => { @@ -84,20 +80,19 @@ export const handleConfigCommand: CommandHandler = async (params, allowTextComma } parsedWritePath = parsedPath.path; const channelId = params.command.channelId ?? normalizeChannelId(params.command.channel); - const writeAuth = authorizeConfigWrite({ + const deniedText = resolveConfigWriteDeniedText({ cfg: params.cfg, - origin: { channelId, accountId: params.ctx.AccountId }, + channel: params.command.channel, + channelId, + accountId: params.ctx.AccountId, + gatewayClientScopes: params.ctx.GatewayClientScopes, target: resolveConfigWriteTargetFromPath(parsedWritePath), - allowBypass: canBypassConfigWritePolicy({ - channel: params.command.channel, - gatewayClientScopes: params.ctx.GatewayClientScopes, - }), }); - if (!writeAuth.allowed) { + if (deniedText) { return { shouldContinue: false, reply: { - text: formatConfigWriteDeniedMessage({ result: writeAuth, fallbackChannelId: channelId }), + text: deniedText, }, }; } diff --git a/src/auto-reply/reply/commands-session-abort.ts b/src/auto-reply/reply/commands-session-abort.ts index e8abdb845d6..2991ede75cd 100644 --- a/src/auto-reply/reply/commands-session-abort.ts +++ b/src/auto-reply/reply/commands-session-abort.ts @@ -86,6 +86,23 @@ async function applyAbortTarget(params: { } } +function buildAbortTargetApplyParams( + params: Parameters[0], + abortTarget: AbortTarget, +) { + return { + abortTarget, + sessionStore: params.sessionStore, + storePath: params.storePath, + abortKey: params.command.abortKey, + abortCutoff: resolveAbortCutoffForTarget({ + ctx: params.ctx, + commandSessionKey: params.sessionKey, + targetSessionKey: abortTarget.key, + }), + }; +} + export const handleStopCommand: CommandHandler = async (params, allowTextCommands) => { if (!allowTextCommands) { return null; @@ -109,17 +126,7 @@ export const handleStopCommand: CommandHandler = async (params, allowTextCommand `stop: cleared followups=${cleared.followupCleared} lane=${cleared.laneCleared} keys=${cleared.keys.join(",")}`, ); } - await applyAbortTarget({ - abortTarget, - sessionStore: params.sessionStore, - storePath: params.storePath, - abortKey: params.command.abortKey, - abortCutoff: resolveAbortCutoffForTarget({ - ctx: params.ctx, - commandSessionKey: params.sessionKey, - targetSessionKey: abortTarget.key, - }), - }); + await applyAbortTarget(buildAbortTargetApplyParams(params, abortTarget)); // Trigger internal hook for stop command const hookEvent = createInternalHookEvent( @@ -160,16 +167,6 @@ export const handleAbortTrigger: CommandHandler = async (params, allowTextComman sessionEntry: params.sessionEntry, sessionStore: params.sessionStore, }); - await applyAbortTarget({ - abortTarget, - sessionStore: params.sessionStore, - storePath: params.storePath, - abortKey: params.command.abortKey, - abortCutoff: resolveAbortCutoffForTarget({ - ctx: params.ctx, - commandSessionKey: params.sessionKey, - targetSessionKey: abortTarget.key, - }), - }); + await applyAbortTarget(buildAbortTargetApplyParams(params, abortTarget)); return { shouldContinue: false, reply: { text: "⚙️ Agent was aborted." } }; }; diff --git a/src/auto-reply/reply/commands-session-lifecycle.test.ts b/src/auto-reply/reply/commands-session-lifecycle.test.ts index 79882f13921..baf5addc60e 100644 --- a/src/auto-reply/reply/commands-session-lifecycle.test.ts +++ b/src/auto-reply/reply/commands-session-lifecycle.test.ts @@ -139,6 +139,21 @@ function createTelegramBinding(overrides?: Partial): Sessi }; } +function expectIdleTimeoutSetReply( + mock: ReturnType, + text: string, + idleTimeoutMs: number, + idleTimeoutLabel: string, +) { + expect(mock).toHaveBeenCalledWith({ + targetSessionKey: "agent:main:subagent:child", + accountId: "default", + idleTimeoutMs, + }); + expect(text).toContain(`Idle timeout set to ${idleTimeoutLabel}`); + expect(text).toContain("2026-02-20T02:00:00.000Z"); +} + function createFakeThreadBindingManager(binding: FakeBinding | null) { return { getByThreadId: vi.fn((_threadId: string) => binding), @@ -175,13 +190,12 @@ describe("/session idle and /session max-age", () => { const result = await handleSessionCommand(createDiscordCommandParams("/session idle 2h"), true); const text = result?.reply?.text ?? ""; - expect(hoisted.setThreadBindingIdleTimeoutBySessionKeyMock).toHaveBeenCalledWith({ - targetSessionKey: "agent:main:subagent:child", - accountId: "default", - idleTimeoutMs: 2 * 60 * 60 * 1000, - }); - expect(text).toContain("Idle timeout set to 2h"); - expect(text).toContain("2026-02-20T02:00:00.000Z"); + expectIdleTimeoutSetReply( + hoisted.setThreadBindingIdleTimeoutBySessionKeyMock, + text, + 2 * 60 * 60 * 1000, + "2h", + ); }); it("shows active idle timeout when no value is provided", async () => { @@ -248,13 +262,12 @@ describe("/session idle and /session max-age", () => { ); const text = result?.reply?.text ?? ""; - expect(hoisted.setTelegramThreadBindingIdleTimeoutBySessionKeyMock).toHaveBeenCalledWith({ - targetSessionKey: "agent:main:subagent:child", - accountId: "default", - idleTimeoutMs: 2 * 60 * 60 * 1000, - }); - expect(text).toContain("Idle timeout set to 2h"); - expect(text).toContain("2026-02-20T02:00:00.000Z"); + expectIdleTimeoutSetReply( + hoisted.setTelegramThreadBindingIdleTimeoutBySessionKeyMock, + text, + 2 * 60 * 60 * 1000, + "2h", + ); }); it("reports Telegram max-age expiry from the original bind time", async () => { diff --git a/src/auto-reply/reply/config-write-authorization.ts b/src/auto-reply/reply/config-write-authorization.ts new file mode 100644 index 00000000000..a2c2142709f --- /dev/null +++ b/src/auto-reply/reply/config-write-authorization.ts @@ -0,0 +1,33 @@ +import { + authorizeConfigWrite, + canBypassConfigWritePolicy, + formatConfigWriteDeniedMessage, +} from "../../channels/plugins/config-writes.js"; +import type { ChannelId } from "../../channels/plugins/types.js"; +import type { OpenClawConfig } from "../../config/config.js"; + +export function resolveConfigWriteDeniedText(params: { + cfg: OpenClawConfig; + channel?: string | null; + channelId: ChannelId | null; + accountId?: string; + gatewayClientScopes?: string[]; + target: Parameters[0]["target"]; +}): string | null { + const writeAuth = authorizeConfigWrite({ + cfg: params.cfg, + origin: { channelId: params.channelId, accountId: params.accountId }, + target: params.target, + allowBypass: canBypassConfigWritePolicy({ + channel: params.channel ?? "", + gatewayClientScopes: params.gatewayClientScopes, + }), + }); + if (writeAuth.allowed) { + return null; + } + return formatConfigWriteDeniedMessage({ + result: writeAuth, + fallbackChannelId: params.channelId, + }); +} diff --git a/src/auto-reply/reply/directive-handling.auth.test.ts b/src/auto-reply/reply/directive-handling.auth.test.ts index 4faad0c3ee6..5e1248c8a61 100644 --- a/src/auto-reply/reply/directive-handling.auth.test.ts +++ b/src/auto-reply/reply/directive-handling.auth.test.ts @@ -4,6 +4,11 @@ import type { OpenClawConfig } from "../../config/config.js"; let mockStore: AuthProfileStore; let mockOrder: string[]; +const githubCopilotTokenRefProfile: AuthProfileStore["profiles"][string] = { + type: "token", + provider: "github-copilot", + tokenRef: { source: "env", provider: "default", id: "GITHUB_TOKEN" }, +}; vi.mock("../../agents/auth-health.js", () => ({ formatRemainingShort: () => "1h", @@ -39,6 +44,28 @@ vi.mock("../../agents/model-auth.js", () => ({ const { resolveAuthLabel } = await import("./directive-handling.auth.js"); +async function resolveRefOnlyAuthLabel(params: { + provider: string; + profileId: string; + profile: + | (AuthProfileStore["profiles"][string] & { type: "api_key" }) + | (AuthProfileStore["profiles"][string] & { type: "token" }); + mode: "compact" | "verbose"; +}) { + mockStore.profiles = { + [params.profileId]: params.profile, + }; + mockOrder = [params.profileId]; + + return resolveAuthLabel( + params.provider, + {} as OpenClawConfig, + "/tmp/models.json", + undefined, + params.mode, + ); +} + describe("resolveAuthLabel ref-aware labels", () => { beforeEach(() => { mockStore = { @@ -49,64 +76,38 @@ describe("resolveAuthLabel ref-aware labels", () => { }); it("shows api-key (ref) for keyRef-only profiles in compact mode", async () => { - mockStore.profiles = { - "openai:default": { + const result = await resolveRefOnlyAuthLabel({ + provider: "openai", + profileId: "openai:default", + profile: { type: "api_key", provider: "openai", keyRef: { source: "env", provider: "default", id: "OPENAI_API_KEY" }, }, - }; - mockOrder = ["openai:default"]; - - const result = await resolveAuthLabel( - "openai", - {} as OpenClawConfig, - "/tmp/models.json", - undefined, - "compact", - ); + mode: "compact", + }); expect(result.label).toBe("openai:default api-key (ref)"); }); it("shows token (ref) for tokenRef-only profiles in compact mode", async () => { - mockStore.profiles = { - "github-copilot:default": { - type: "token", - provider: "github-copilot", - tokenRef: { source: "env", provider: "default", id: "GITHUB_TOKEN" }, - }, - }; - mockOrder = ["github-copilot:default"]; - - const result = await resolveAuthLabel( - "github-copilot", - {} as OpenClawConfig, - "/tmp/models.json", - undefined, - "compact", - ); + const result = await resolveRefOnlyAuthLabel({ + provider: "github-copilot", + profileId: "github-copilot:default", + profile: githubCopilotTokenRefProfile, + mode: "compact", + }); expect(result.label).toBe("github-copilot:default token (ref)"); }); it("uses token:ref instead of token:missing in verbose mode", async () => { - mockStore.profiles = { - "github-copilot:default": { - type: "token", - provider: "github-copilot", - tokenRef: { source: "env", provider: "default", id: "GITHUB_TOKEN" }, - }, - }; - mockOrder = ["github-copilot:default"]; - - const result = await resolveAuthLabel( - "github-copilot", - {} as OpenClawConfig, - "/tmp/models.json", - undefined, - "verbose", - ); + const result = await resolveRefOnlyAuthLabel({ + provider: "github-copilot", + profileId: "github-copilot:default", + profile: githubCopilotTokenRefProfile, + mode: "verbose", + }); expect(result.label).toContain("github-copilot:default=token:ref"); expect(result.label).not.toContain("token:missing"); diff --git a/src/auto-reply/reply/directive-handling.auth.ts b/src/auto-reply/reply/directive-handling.auth.ts index 26647d77c68..604e7473ae8 100644 --- a/src/auto-reply/reply/directive-handling.auth.ts +++ b/src/auto-reply/reply/directive-handling.auth.ts @@ -33,6 +33,22 @@ function resolveStoredCredentialLabel(params: { return "missing"; } +function formatExpirationLabel( + expires: unknown, + now: number, + formatUntil: (timestampMs: number) => string, + compactExpiredPrefix = " expired", +) { + if (typeof expires !== "number" || !Number.isFinite(expires) || expires <= 0) { + return ""; + } + return expires <= now ? compactExpiredPrefix : ` exp ${formatUntil(expires)}`; +} + +function formatFlagsSuffix(flags: string[]) { + return flags.length > 0 ? ` (${flags.join(", ")})` : ""; +} + export const resolveAuthLabel = async ( provider: string, cfg: OpenClawConfig, @@ -89,14 +105,7 @@ export const resolveAuthLabel = async ( refValue: profile.tokenRef, mode, }); - const exp = - typeof profile.expires === "number" && - Number.isFinite(profile.expires) && - profile.expires > 0 - ? profile.expires <= now - ? " expired" - : ` exp ${formatUntil(profile.expires)}` - : ""; + const exp = formatExpirationLabel(profile.expires, now, formatUntil); return { label: `${profileId} token ${tokenLabel}${exp}${more}`, source: "", @@ -104,14 +113,7 @@ export const resolveAuthLabel = async ( } const display = resolveAuthProfileDisplayLabel({ cfg, store, profileId }); const label = display === profileId ? profileId : display; - const exp = - typeof profile.expires === "number" && - Number.isFinite(profile.expires) && - profile.expires > 0 - ? profile.expires <= now - ? " expired" - : ` exp ${formatUntil(profile.expires)}` - : ""; + const exp = formatExpirationLabel(profile.expires, now, formatUntil); return { label: `${label} oauth${exp}${more}`, source: "" }; } @@ -140,7 +142,7 @@ export const resolveAuthLabel = async ( configProfile.mode !== profile.type && !(configProfile.mode === "oauth" && profile.type === "token")) ) { - const suffix = flags.length > 0 ? ` (${flags.join(", ")})` : ""; + const suffix = formatFlagsSuffix(flags); return `${profileId}=missing${suffix}`; } if (profile.type === "api_key") { @@ -149,7 +151,7 @@ export const resolveAuthLabel = async ( refValue: profile.keyRef, mode, }); - const suffix = flags.length > 0 ? ` (${flags.join(", ")})` : ""; + const suffix = formatFlagsSuffix(flags); return `${profileId}=${keyLabel}${suffix}`; } if (profile.type === "token") { @@ -158,14 +160,11 @@ export const resolveAuthLabel = async ( refValue: profile.tokenRef, mode, }); - if ( - typeof profile.expires === "number" && - Number.isFinite(profile.expires) && - profile.expires > 0 - ) { - flags.push(profile.expires <= now ? "expired" : `exp ${formatUntil(profile.expires)}`); + const expirationFlag = formatExpirationLabel(profile.expires, now, formatUntil, "expired"); + if (expirationFlag) { + flags.push(expirationFlag); } - const suffix = flags.length > 0 ? ` (${flags.join(", ")})` : ""; + const suffix = formatFlagsSuffix(flags); return `${profileId}=token:${tokenLabel}${suffix}`; } const display = resolveAuthProfileDisplayLabel({ @@ -179,15 +178,12 @@ export const resolveAuthLabel = async ( : display.startsWith(profileId) ? display.slice(profileId.length).trim() : `(${display})`; - if ( - typeof profile.expires === "number" && - Number.isFinite(profile.expires) && - profile.expires > 0 - ) { - flags.push(profile.expires <= now ? "expired" : `exp ${formatUntil(profile.expires)}`); + const expirationFlag = formatExpirationLabel(profile.expires, now, formatUntil, "expired"); + if (expirationFlag) { + flags.push(expirationFlag); } const suffixLabel = suffix ? ` ${suffix}` : ""; - const suffixFlags = flags.length > 0 ? ` (${flags.join(", ")})` : ""; + const suffixFlags = formatFlagsSuffix(flags); return `${profileId}=OAuth${suffixLabel}${suffixFlags}`; }); return { diff --git a/src/auto-reply/reply/directive-handling.model.test.ts b/src/auto-reply/reply/directive-handling.model.test.ts index 5d4a23f3efb..b815ecfc9b9 100644 --- a/src/auto-reply/reply/directive-handling.model.test.ts +++ b/src/auto-reply/reply/directive-handling.model.test.ts @@ -57,24 +57,28 @@ function resolveModelSelectionForCommand(params: { }); } +async function resolveModelInfoReply( + overrides: Partial[0]> = {}, +) { + return maybeHandleModelDirectiveInfo({ + directives: parseInlineDirectives("/model"), + cfg: baseConfig(), + agentDir: "/tmp/agent", + activeAgentId: "main", + provider: "anthropic", + model: "claude-opus-4-5", + defaultProvider: "anthropic", + defaultModel: "claude-opus-4-5", + aliasIndex: baseAliasIndex(), + allowedModelCatalog: [], + resetModelOverride: false, + ...overrides, + }); +} + describe("/model chat UX", () => { it("shows summary for /model with no args", async () => { - const directives = parseInlineDirectives("/model"); - const cfg = { commands: { text: true } } as unknown as OpenClawConfig; - - const reply = await maybeHandleModelDirectiveInfo({ - directives, - cfg, - agentDir: "/tmp/agent", - activeAgentId: "main", - provider: "anthropic", - model: "claude-opus-4-5", - defaultProvider: "anthropic", - defaultModel: "claude-opus-4-5", - aliasIndex: baseAliasIndex(), - allowedModelCatalog: [], - resetModelOverride: false, - }); + const reply = await resolveModelInfoReply(); expect(reply?.text).toContain("Current:"); expect(reply?.text).toContain("Browse: /models"); @@ -82,21 +86,11 @@ describe("/model chat UX", () => { }); it("shows active runtime model when different from selected model", async () => { - const directives = parseInlineDirectives("/model"); - const cfg = { commands: { text: true } } as unknown as OpenClawConfig; - - const reply = await maybeHandleModelDirectiveInfo({ - directives, - cfg, - agentDir: "/tmp/agent", - activeAgentId: "main", + const reply = await resolveModelInfoReply({ provider: "fireworks", model: "fireworks/minimax-m2p5", defaultProvider: "fireworks", defaultModel: "fireworks/minimax-m2p5", - aliasIndex: baseAliasIndex(), - allowedModelCatalog: [], - resetModelOverride: false, sessionEntry: { modelProvider: "deepinfra", model: "moonshotai/Kimi-K2.5", diff --git a/src/auto-reply/reply/discord-parent-channel.ts b/src/auto-reply/reply/discord-parent-channel.ts new file mode 100644 index 00000000000..877c4593ea7 --- /dev/null +++ b/src/auto-reply/reply/discord-parent-channel.ts @@ -0,0 +1,15 @@ +import { normalizeConversationText } from "../../acp/conversation-id.js"; +import { parseAgentSessionKey } from "../../routing/session-key.js"; + +export function parseDiscordParentChannelFromSessionKey(raw: unknown): string | undefined { + const sessionKey = normalizeConversationText(raw); + if (!sessionKey) { + return undefined; + } + const scoped = parseAgentSessionKey(sessionKey)?.rest ?? sessionKey.toLowerCase(); + const match = scoped.match(/(?:^|:)channel:([^:]+)$/); + if (!match?.[1]) { + return undefined; + } + return match[1]; +} diff --git a/src/auto-reply/reply/followup-runner.test.ts b/src/auto-reply/reply/followup-runner.test.ts index a02ce0b2038..8d12e815685 100644 --- a/src/auto-reply/reply/followup-runner.test.ts +++ b/src/auto-reply/reply/followup-runner.test.ts @@ -4,7 +4,7 @@ import path from "node:path"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { loadSessionStore, saveSessionStore, type SessionEntry } from "../../config/sessions.js"; import type { FollowupRun } from "./queue.js"; -import { createMockTypingController } from "./test-helpers.js"; +import { createMockFollowupRun, createMockTypingController } from "./test-helpers.js"; const runEmbeddedPiAgentMock = vi.fn(); const routeReplyMock = vi.fn(); @@ -50,47 +50,12 @@ beforeEach(() => { }); const baseQueuedRun = (messageProvider = "whatsapp"): FollowupRun => - ({ - prompt: "hello", - summaryLine: "hello", - enqueuedAt: Date.now(), - originatingTo: "channel:C1", - run: { - sessionId: "session", - sessionKey: "main", - messageProvider, - agentAccountId: "primary", - sessionFile: "/tmp/session.jsonl", - workspaceDir: "/tmp", - config: {}, - skillsSnapshot: {}, - provider: "anthropic", - model: "claude", - thinkLevel: "low", - verboseLevel: "off", - elevatedLevel: "off", - bashElevated: { - enabled: false, - allowed: false, - defaultLevel: "off", - }, - timeoutMs: 1_000, - blockReplyBreak: "message_end", - }, - }) as FollowupRun; + createMockFollowupRun({ run: { messageProvider } }); function createQueuedRun( overrides: Partial> & { run?: Partial } = {}, ): FollowupRun { - const base = baseQueuedRun(); - return { - ...base, - ...overrides, - run: { - ...base.run, - ...overrides.run, - }, - }; + return createMockFollowupRun(overrides); } function mockCompactionRun(params: { diff --git a/src/auto-reply/reply/get-reply-inline-actions.skip-when-config-empty.test.ts b/src/auto-reply/reply/get-reply-inline-actions.skip-when-config-empty.test.ts index 51351f05de8..36b5910ecae 100644 --- a/src/auto-reply/reply/get-reply-inline-actions.skip-when-config-empty.test.ts +++ b/src/auto-reply/reply/get-reply-inline-actions.skip-when-config-empty.test.ts @@ -84,6 +84,19 @@ const createHandleInlineActionsInput = (params: { }; }; +async function expectInlineActionSkipped(params: { + ctx: ReturnType; + typing: TypingController; + cleanedBody: string; + command?: Partial; + overrides?: Partial>; +}) { + const result = await handleInlineActions(createHandleInlineActionsInput(params)); + expect(result).toEqual({ kind: "reply", reply: undefined }); + expect(params.typing.cleanup).toHaveBeenCalled(); + expect(handleCommandsMock).not.toHaveBeenCalled(); +} + describe("handleInlineActions", () => { beforeEach(() => { handleCommandsMock.mockReset(); @@ -97,18 +110,12 @@ describe("handleInlineActions", () => { To: "whatsapp:+123", Body: "hi", }); - const result = await handleInlineActions( - createHandleInlineActionsInput({ - ctx, - typing, - cleanedBody: "hi", - command: { to: "whatsapp:+123" }, - }), - ); - - expect(result).toEqual({ kind: "reply", reply: undefined }); - expect(typing.cleanup).toHaveBeenCalled(); - expect(handleCommandsMock).not.toHaveBeenCalled(); + await expectInlineActionSkipped({ + ctx, + typing, + cleanedBody: "hi", + command: { to: "whatsapp:+123" }, + }); }); it("forwards agentDir into handleCommands", async () => { @@ -163,25 +170,19 @@ describe("handleInlineActions", () => { MessageSid: "41", }); - const result = await handleInlineActions( - createHandleInlineActionsInput({ - ctx, - typing, - cleanedBody: "old queued message", - command: { - rawBodyNormalized: "old queued message", - commandBodyNormalized: "old queued message", - }, - overrides: { - sessionEntry, - sessionStore, - }, - }), - ); - - expect(result).toEqual({ kind: "reply", reply: undefined }); - expect(typing.cleanup).toHaveBeenCalled(); - expect(handleCommandsMock).not.toHaveBeenCalled(); + await expectInlineActionSkipped({ + ctx, + typing, + cleanedBody: "old queued message", + command: { + rawBodyNormalized: "old queued message", + commandBodyNormalized: "old queued message", + }, + overrides: { + sessionEntry, + sessionStore, + }, + }); }); it("clears /stop cutoff when a newer message arrives", async () => { diff --git a/src/auto-reply/reply/post-compaction-context.test.ts b/src/auto-reply/reply/post-compaction-context.test.ts index 02a4a27e6de..3af8bceab00 100644 --- a/src/auto-reply/reply/post-compaction-context.test.ts +++ b/src/auto-reply/reply/post-compaction-context.test.ts @@ -15,6 +15,28 @@ describe("readPostCompactionContext", () => { fs.rmSync(tmpDir, { recursive: true, force: true }); }); + async function expectLegacySectionFallback( + postCompactionSections: string[], + expectDefaultProse = false, + ) { + const content = `## Every Session\n\nDo startup things.\n\n## Safety\n\nBe safe.\n`; + fs.writeFileSync(path.join(tmpDir, "AGENTS.md"), content); + const cfg = { + agents: { + defaults: { + compaction: { postCompactionSections }, + }, + }, + } as OpenClawConfig; + const result = await readPostCompactionContext(tmpDir, cfg); + expect(result).not.toBeNull(); + expect(result).toContain("Do startup things"); + expect(result).toContain("Be safe"); + if (expectDefaultProse) { + expect(result).toContain("Run your Session Startup sequence"); + } + } + it("returns null when no AGENTS.md exists", async () => { const result = await readPostCompactionContext(tmpDir); expect(result).toBeNull(); @@ -339,36 +361,11 @@ Read WORKFLOW.md on startup. // Older AGENTS.md templates use "Every Session" / "Safety" instead of // "Session Startup" / "Red Lines". Explicitly setting the defaults should // still trigger the legacy fallback — same behavior as leaving the field unset. - const content = `## Every Session\n\nDo startup things.\n\n## Safety\n\nBe safe.\n`; - fs.writeFileSync(path.join(tmpDir, "AGENTS.md"), content); - const cfg = { - agents: { - defaults: { - compaction: { postCompactionSections: ["Session Startup", "Red Lines"] }, - }, - }, - } as OpenClawConfig; - const result = await readPostCompactionContext(tmpDir, cfg); - expect(result).not.toBeNull(); - expect(result).toContain("Do startup things"); - expect(result).toContain("Be safe"); + await expectLegacySectionFallback(["Session Startup", "Red Lines"]); }); it("falls back to legacy sections when default sections are configured in a different order", async () => { - const content = `## Every Session\n\nDo startup things.\n\n## Safety\n\nBe safe.\n`; - fs.writeFileSync(path.join(tmpDir, "AGENTS.md"), content); - const cfg = { - agents: { - defaults: { - compaction: { postCompactionSections: ["Red Lines", "Session Startup"] }, - }, - }, - } as OpenClawConfig; - const result = await readPostCompactionContext(tmpDir, cfg); - expect(result).not.toBeNull(); - expect(result).toContain("Do startup things"); - expect(result).toContain("Be safe"); - expect(result).toContain("Run your Session Startup sequence"); + await expectLegacySectionFallback(["Red Lines", "Session Startup"], true); }); it("custom section names are matched case-insensitively", async () => { diff --git a/src/auto-reply/reply/reply-elevated.test.ts b/src/auto-reply/reply/reply-elevated.test.ts index 74fba60acf7..28259c34638 100644 --- a/src/auto-reply/reply/reply-elevated.test.ts +++ b/src/auto-reply/reply/reply-elevated.test.ts @@ -27,68 +27,65 @@ function buildContext(overrides?: Partial): MsgContext { } as MsgContext; } +function expectAllowFromDecision(params: { + allowFrom: string[]; + ctx?: Partial; + allowed: boolean; +}) { + const result = resolveElevatedPermissions({ + cfg: buildConfig(params.allowFrom), + agentId: "main", + provider: "whatsapp", + ctx: buildContext(params.ctx), + }); + + expect(result.enabled).toBe(true); + expect(result.allowed).toBe(params.allowed); + if (params.allowed) { + expect(result.failures).toHaveLength(0); + return; + } + + expect(result.failures).toContainEqual({ + gate: "allowFrom", + key: "tools.elevated.allowFrom.whatsapp", + }); +} + describe("resolveElevatedPermissions", () => { it("authorizes when sender matches allowFrom", () => { - const result = resolveElevatedPermissions({ - cfg: buildConfig(["+15550001111"]), - agentId: "main", - provider: "whatsapp", - ctx: buildContext(), + expectAllowFromDecision({ + allowFrom: ["+15550001111"], + allowed: true, }); - - expect(result.enabled).toBe(true); - expect(result.allowed).toBe(true); - expect(result.failures).toHaveLength(0); }); it("does not authorize when only recipient matches allowFrom", () => { - const result = resolveElevatedPermissions({ - cfg: buildConfig(["+15559990000"]), - agentId: "main", - provider: "whatsapp", - ctx: buildContext(), - }); - - expect(result.enabled).toBe(true); - expect(result.allowed).toBe(false); - expect(result.failures).toContainEqual({ - gate: "allowFrom", - key: "tools.elevated.allowFrom.whatsapp", + expectAllowFromDecision({ + allowFrom: ["+15559990000"], + allowed: false, }); }); it("does not authorize untyped mutable sender fields", () => { - const result = resolveElevatedPermissions({ - cfg: buildConfig(["owner-display-name"]), - agentId: "main", - provider: "whatsapp", - ctx: buildContext({ + expectAllowFromDecision({ + allowFrom: ["owner-display-name"], + allowed: false, + ctx: { SenderName: "owner-display-name", SenderUsername: "owner-display-name", SenderTag: "owner-display-name", - }), - }); - - expect(result.enabled).toBe(true); - expect(result.allowed).toBe(false); - expect(result.failures).toContainEqual({ - gate: "allowFrom", - key: "tools.elevated.allowFrom.whatsapp", + }, }); }); it("authorizes mutable sender fields only with explicit prefix", () => { - const result = resolveElevatedPermissions({ - cfg: buildConfig(["username:owner_username"]), - agentId: "main", - provider: "whatsapp", - ctx: buildContext({ + expectAllowFromDecision({ + allowFrom: ["username:owner_username"], + allowed: true, + ctx: { SenderUsername: "owner_username", - }), + }, }); - - expect(result.enabled).toBe(true); - expect(result.allowed).toBe(true); - expect(result.failures).toHaveLength(0); }); }); diff --git a/src/auto-reply/reply/route-reply.test.ts b/src/auto-reply/reply/route-reply.test.ts index 5a0405da22b..62f91097223 100644 --- a/src/auto-reply/reply/route-reply.test.ts +++ b/src/auto-reply/reply/route-reply.test.ts @@ -105,6 +105,23 @@ const createMSTeamsPlugin = (params: { outbound: ChannelOutboundAdapter }): Chan outbound: params.outbound, }); +async function expectSlackNoSend( + payload: Parameters[0]["payload"], + overrides: Partial[0]> = {}, +) { + mocks.sendMessageSlack.mockClear(); + const res = await routeReply({ + payload, + channel: "slack", + to: "channel:C123", + cfg: {} as never, + ...overrides, + }); + expect(res.ok).toBe(true); + expect(mocks.sendMessageSlack).not.toHaveBeenCalled(); + return res; +} + describe("routeReply", () => { beforeEach(() => { setActivePluginRegistry(defaultRegistry); @@ -132,39 +149,15 @@ describe("routeReply", () => { }); it("no-ops on empty payload", async () => { - mocks.sendMessageSlack.mockClear(); - const res = await routeReply({ - payload: {}, - channel: "slack", - to: "channel:C123", - cfg: {} as never, - }); - expect(res.ok).toBe(true); - expect(mocks.sendMessageSlack).not.toHaveBeenCalled(); + await expectSlackNoSend({}); }); it("suppresses reasoning payloads", async () => { - mocks.sendMessageSlack.mockClear(); - const res = await routeReply({ - payload: { text: "Reasoning:\n_step_", isReasoning: true }, - channel: "slack", - to: "channel:C123", - cfg: {} as never, - }); - expect(res.ok).toBe(true); - expect(mocks.sendMessageSlack).not.toHaveBeenCalled(); + await expectSlackNoSend({ text: "Reasoning:\n_step_", isReasoning: true }); }); it("drops silent token payloads", async () => { - mocks.sendMessageSlack.mockClear(); - const res = await routeReply({ - payload: { text: SILENT_REPLY_TOKEN }, - channel: "slack", - to: "channel:C123", - cfg: {} as never, - }); - expect(res.ok).toBe(true); - expect(mocks.sendMessageSlack).not.toHaveBeenCalled(); + await expectSlackNoSend({ text: SILENT_REPLY_TOKEN }); }); it("does not drop payloads that merely start with the silent token", async () => { @@ -231,23 +224,14 @@ describe("routeReply", () => { }); it("does not bypass the empty-reply guard for invalid Slack blocks", async () => { - mocks.sendMessageSlack.mockClear(); - const res = await routeReply({ - payload: { - text: " ", - channelData: { - slack: { - blocks: " ", - }, + await expectSlackNoSend({ + text: " ", + channelData: { + slack: { + blocks: " ", }, }, - channel: "slack", - to: "channel:C123", - cfg: {} as never, }); - - expect(res.ok).toBe(true); - expect(mocks.sendMessageSlack).not.toHaveBeenCalled(); }); it("does not derive responsePrefix from agent identity when routing", async () => { diff --git a/src/auto-reply/reply/session-updates.ts b/src/auto-reply/reply/session-updates.ts index 96243e919bb..55b4d4eb15b 100644 --- a/src/auto-reply/reply/session-updates.ts +++ b/src/auto-reply/reply/session-updates.ts @@ -117,6 +117,27 @@ export async function drainFormattedSystemEvents(params: { .join("\n"); } +async function persistSessionEntryUpdate(params: { + sessionStore?: Record; + sessionKey?: string; + storePath?: string; + nextEntry: SessionEntry; +}) { + if (!params.sessionStore || !params.sessionKey) { + return; + } + params.sessionStore[params.sessionKey] = { + ...params.sessionStore[params.sessionKey], + ...params.nextEntry, + }; + if (!params.storePath) { + return; + } + await updateSessionStore(params.storePath, (store) => { + store[params.sessionKey!] = { ...store[params.sessionKey!], ...params.nextEntry }; + }); +} + export async function ensureSkillSnapshot(params: { sessionEntry?: SessionEntry; sessionStore?: Record; @@ -185,12 +206,7 @@ export async function ensureSkillSnapshot(params: { systemSent: true, skillsSnapshot: skillSnapshot, }; - sessionStore[sessionKey] = { ...sessionStore[sessionKey], ...nextEntry }; - if (storePath) { - await updateSessionStore(storePath, (store) => { - store[sessionKey] = { ...store[sessionKey], ...nextEntry }; - }); - } + await persistSessionEntryUpdate({ sessionStore, sessionKey, storePath, nextEntry }); systemSent = true; } @@ -227,12 +243,7 @@ export async function ensureSkillSnapshot(params: { updatedAt: Date.now(), skillsSnapshot, }; - sessionStore[sessionKey] = { ...sessionStore[sessionKey], ...nextEntry }; - if (storePath) { - await updateSessionStore(storePath, (store) => { - store[sessionKey] = { ...store[sessionKey], ...nextEntry }; - }); - } + await persistSessionEntryUpdate({ sessionStore, sessionKey, storePath, nextEntry }); } return { sessionEntry: nextEntry, skillsSnapshot, systemSent }; diff --git a/src/auto-reply/reply/session.ts b/src/auto-reply/reply/session.ts index 85e6754025f..a2c0b1c7cf4 100644 --- a/src/auto-reply/reply/session.ts +++ b/src/auto-reply/reply/session.ts @@ -34,11 +34,12 @@ import { resolveConversationIdFromTargets } from "../../infra/outbound/conversat import { deliverSessionMaintenanceWarning } from "../../infra/session-maintenance-warning.js"; import { createSubsystemLogger } from "../../logging/subsystem.js"; import { getGlobalHookRunner } from "../../plugins/hook-runner-global.js"; -import { normalizeMainKey, parseAgentSessionKey } from "../../routing/session-key.js"; +import { normalizeMainKey } from "../../routing/session-key.js"; import { normalizeSessionDeliveryFields } from "../../utils/delivery-context.js"; import { resolveCommandAuthorization } from "../command-auth.js"; import type { MsgContext, TemplateContext } from "../templating.js"; import { resolveEffectiveResetTargetSessionKey } from "./acp-reset-target.js"; +import { parseDiscordParentChannelFromSessionKey } from "./discord-parent-channel.js"; import { normalizeInboundTextNewlines } from "./inbound-text.js"; import { stripMentions, stripStructuralPrefixes } from "./mentions.js"; import { @@ -70,19 +71,6 @@ export type SessionInitResult = { triggerBodyNormalized: string; }; -function parseDiscordParentChannelFromSessionKey(raw: unknown): string | undefined { - const sessionKey = normalizeConversationText(raw); - if (!sessionKey) { - return undefined; - } - const scoped = parseAgentSessionKey(sessionKey)?.rest ?? sessionKey.toLowerCase(); - const match = scoped.match(/(?:^|:)channel:([^:]+)$/); - if (!match?.[1]) { - return undefined; - } - return match[1]; -} - function resolveAcpResetBindingContext(ctx: MsgContext): { channel: string; accountId: string; diff --git a/src/auto-reply/reply/stage-sandbox-media.ts b/src/auto-reply/reply/stage-sandbox-media.ts index d364fa6a554..3d3dec1738f 100644 --- a/src/auto-reply/reply/stage-sandbox-media.ts +++ b/src/auto-reply/reply/stage-sandbox-media.ts @@ -7,7 +7,7 @@ import { ensureSandboxWorkspaceForSession } from "../../agents/sandbox.js"; import type { OpenClawConfig } from "../../config/config.js"; import { logVerbose } from "../../globals.js"; import { copyFileWithinRoot, SafeOpenError } from "../../infra/fs-safe.js"; -import { normalizeScpRemoteHost } from "../../infra/scp-host.js"; +import { normalizeScpRemoteHost, normalizeScpRemotePath } from "../../infra/scp-host.js"; import { resolvePreferredOpenClawTmpDir } from "../../infra/tmp-openclaw-dir.js"; import { isInboundPathAllowed, @@ -293,6 +293,10 @@ async function scpFile(remoteHost: string, remotePath: string, localPath: string if (!safeRemoteHost) { throw new Error("invalid remote host for SCP"); } + const safeRemotePath = normalizeScpRemotePath(remotePath); + if (!safeRemotePath) { + throw new Error("invalid remote path for SCP"); + } return new Promise((resolve, reject) => { const child = spawn( "/usr/bin/scp", @@ -302,7 +306,7 @@ async function scpFile(remoteHost: string, remotePath: string, localPath: string "-o", "StrictHostKeyChecking=yes", "--", - `${safeRemoteHost}:${remotePath}`, + `${safeRemoteHost}:${safeRemotePath}`, localPath, ], { stdio: ["ignore", "ignore", "pipe"] }, diff --git a/src/auto-reply/reply/test-helpers.ts b/src/auto-reply/reply/test-helpers.ts index 4c30ae6756a..d92bf481f42 100644 --- a/src/auto-reply/reply/test-helpers.ts +++ b/src/auto-reply/reply/test-helpers.ts @@ -1,4 +1,5 @@ import { vi } from "vitest"; +import type { FollowupRun } from "./queue.js"; import type { TypingController } from "./typing.js"; export function createMockTypingController( @@ -16,3 +17,44 @@ export function createMockTypingController( ...overrides, }; } + +export function createMockFollowupRun( + overrides: Partial> & { run?: Partial } = {}, +): FollowupRun { + const base: FollowupRun = { + prompt: "hello", + summaryLine: "hello", + enqueuedAt: Date.now(), + originatingTo: "channel:C1", + run: { + sessionId: "session", + sessionKey: "main", + messageProvider: "whatsapp", + agentAccountId: "primary", + sessionFile: "/tmp/session.jsonl", + workspaceDir: "/tmp", + config: {}, + skillsSnapshot: {}, + provider: "anthropic", + model: "claude", + thinkLevel: "low", + verboseLevel: "off", + elevatedLevel: "off", + bashElevated: { + enabled: false, + allowed: false, + defaultLevel: "off", + }, + timeoutMs: 1_000, + blockReplyBreak: "message_end", + }, + }; + return { + ...base, + ...overrides, + run: { + ...base.run, + ...overrides.run, + }, + }; +} diff --git a/src/browser/chrome-mcp.test.ts b/src/browser/chrome-mcp.test.ts index ec6f21339ed..a9571f31205 100644 --- a/src/browser/chrome-mcp.test.ts +++ b/src/browser/chrome-mcp.test.ts @@ -129,4 +129,64 @@ describe("chrome MCP page parsing", () => { expect(result).toBe(123); }); + + it("surfaces MCP tool errors instead of JSON parse noise", async () => { + const factory: ChromeMcpSessionFactory = async () => { + const session = createFakeSession(); + const callTool = vi.fn(async ({ name }: ToolCall) => { + if (name === "evaluate_script") { + return { + content: [ + { + type: "text", + text: "Cannot read properties of null (reading 'value')", + }, + ], + isError: true, + }; + } + throw new Error(`unexpected tool ${name}`); + }); + session.client.callTool = callTool as typeof session.client.callTool; + return session; + }; + setChromeMcpSessionFactoryForTest(factory); + + await expect( + evaluateChromeMcpScript({ + profileName: "chrome-live", + targetId: "1", + fn: "() => document.getElementById('missing').value", + }), + ).rejects.toThrow(/Cannot read properties of null/); + }); + + it("reuses a single pending session for concurrent requests", async () => { + let factoryCalls = 0; + let releaseFactory!: () => void; + const factoryGate = new Promise((resolve) => { + releaseFactory = resolve; + }); + + const factory: ChromeMcpSessionFactory = async () => { + factoryCalls += 1; + await factoryGate; + return createFakeSession(); + }; + setChromeMcpSessionFactoryForTest(factory); + + const tabsPromise = listChromeMcpTabs("chrome-live"); + const evalPromise = evaluateChromeMcpScript({ + profileName: "chrome-live", + targetId: "1", + fn: "() => 123", + }); + + releaseFactory(); + const [tabs, result] = await Promise.all([tabsPromise, evalPromise]); + + expect(factoryCalls).toBe(1); + expect(tabs).toHaveLength(2); + expect(result).toBe(123); + }); }); diff --git a/src/browser/chrome-mcp.ts b/src/browser/chrome-mcp.ts index ecd196547d3..e410cf886e9 100644 --- a/src/browser/chrome-mcp.ts +++ b/src/browser/chrome-mcp.ts @@ -39,6 +39,7 @@ const DEFAULT_CHROME_MCP_ARGS = [ ]; const sessions = new Map(); +const pendingSessions = new Map>(); let sessionFactory: ChromeMcpSessionFactory | null = null; function asRecord(value: unknown): Record | null { @@ -144,6 +145,11 @@ function extractMessageText(result: ChromeMcpToolResult): string { return blocks.find((block) => block.trim()) ?? ""; } +function extractToolErrorMessage(result: ChromeMcpToolResult, name: string): string { + const message = extractMessageText(result).trim(); + return message || `Chrome MCP tool "${name}" failed.`; +} + function extractJsonMessage(result: ChromeMcpToolResult): unknown { const candidates = [extractMessageText(result), ...extractTextContent(result)].filter((text) => text.trim(), @@ -207,8 +213,22 @@ async function getSession(profileName: string): Promise { session = undefined; } if (!session) { - session = await (sessionFactory ?? createRealSession)(profileName); - sessions.set(profileName, session); + let pending = pendingSessions.get(profileName); + if (!pending) { + pending = (async () => { + const created = await (sessionFactory ?? createRealSession)(profileName); + sessions.set(profileName, created); + return created; + })(); + pendingSessions.set(profileName, pending); + } + try { + session = await pending; + } finally { + if (pendingSessions.get(profileName) === pending) { + pendingSessions.delete(profileName); + } + } } try { await session.ready; @@ -229,10 +249,14 @@ async function callTool( ): Promise { const session = await getSession(profileName); try { - return (await session.client.callTool({ + const result = (await session.client.callTool({ name, arguments: args, })) as ChromeMcpToolResult; + if (result.isError) { + throw new Error(extractToolErrorMessage(result, name)); + } + return result; } catch (err) { sessions.delete(profileName); await session.client.close().catch(() => {}); @@ -268,6 +292,7 @@ export function getChromeMcpPid(profileName: string): number | null { } export async function closeChromeMcpSession(profileName: string): Promise { + pendingSessions.delete(profileName); const session = sessions.get(profileName); if (!session) { return false; @@ -508,5 +533,6 @@ export function setChromeMcpSessionFactoryForTest(factory: ChromeMcpSessionFacto export async function resetChromeMcpSessionsForTest(): Promise { sessionFactory = null; + pendingSessions.clear(); await stopAllChromeMcpSessions(); } diff --git a/src/browser/pw-tools-core.responses.ts b/src/browser/pw-tools-core.responses.ts index 5a6ddc1818c..4b153692a20 100644 --- a/src/browser/pw-tools-core.responses.ts +++ b/src/browser/pw-tools-core.responses.ts @@ -1,22 +1,7 @@ import { formatCliCommand } from "../cli/command-format.js"; import { ensurePageState, getPageForTargetId } from "./pw-session.js"; import { normalizeTimeoutMs } from "./pw-tools-core.shared.js"; - -function matchUrlPattern(pattern: string, url: string): boolean { - const p = pattern.trim(); - if (!p) { - return false; - } - if (p === url) { - return true; - } - if (p.includes("*")) { - const escaped = p.replace(/[|\\{}()[\]^$+?.]/g, "\\$&"); - const regex = new RegExp(`^${escaped.replace(/\*\*/g, ".*").replace(/\*/g, ".*")}$`); - return regex.test(url); - } - return url.includes(p); -} +import { matchBrowserUrlPattern } from "./url-pattern.js"; export async function responseBodyViaPlaywright(opts: { cdpUrl: string; @@ -65,7 +50,7 @@ export async function responseBodyViaPlaywright(opts: { } const r = resp as { url?: () => string }; const u = r.url?.() || ""; - if (!matchUrlPattern(pattern, u)) { + if (!matchBrowserUrlPattern(pattern, u)) { return; } done = true; diff --git a/src/browser/pw-tools-core.waits-next-download-saves-it.test.ts b/src/browser/pw-tools-core.waits-next-download-saves-it.test.ts index d976f7d7fb8..e5aa5bac2e0 100644 --- a/src/browser/pw-tools-core.waits-next-download-saves-it.test.ts +++ b/src/browser/pw-tools-core.waits-next-download-saves-it.test.ts @@ -291,6 +291,6 @@ describe("pw-tools-core", () => { targetId: "T1", ref: " ", }), - ).rejects.toThrow(/ref is required/i); + ).rejects.toThrow(/ref or selector is required/i); }); }); diff --git a/src/browser/routes/agent.act.ts b/src/browser/routes/agent.act.ts index a8d3a09ce83..ae18c044265 100644 --- a/src/browser/routes/agent.act.ts +++ b/src/browser/routes/agent.act.ts @@ -12,6 +12,7 @@ import { import type { BrowserActRequest, BrowserFormField } from "../client-actions-core.js"; import { normalizeBrowserFormField } from "../form-fields.js"; import type { BrowserRouteContext } from "../server-context.js"; +import { matchBrowserUrlPattern } from "../url-pattern.js"; import { registerBrowserAgentActDownloadRoutes } from "./agent.act.download.js"; import { registerBrowserAgentActHookRoutes } from "./agent.act.hooks.js"; import { @@ -47,7 +48,6 @@ function buildExistingSessionWaitPredicate(params: { text?: string; textGone?: string; selector?: string; - url?: string; loadState?: "load" | "domcontentloaded" | "networkidle"; fn?: string; }): string | null { @@ -61,9 +61,6 @@ function buildExistingSessionWaitPredicate(params: { if (params.selector) { checks.push(`Boolean(document.querySelector(${JSON.stringify(params.selector)}))`); } - if (params.url) { - checks.push(`window.location.href === ${JSON.stringify(params.url)}`); - } if (params.loadState === "domcontentloaded") { checks.push(`document.readyState === "interactive" || document.readyState === "complete"`); } else if (params.loadState === "load") { @@ -94,17 +91,30 @@ async function waitForExistingSessionCondition(params: { await sleep(params.timeMs); } const predicate = buildExistingSessionWaitPredicate(params); - if (!predicate) { + if (!predicate && !params.url) { return; } const timeoutMs = Math.max(250, params.timeoutMs ?? 10_000); const deadline = Date.now() + timeoutMs; while (Date.now() < deadline) { - const ready = await evaluateChromeMcpScript({ - profileName: params.profileName, - targetId: params.targetId, - fn: `async () => ${predicate}`, - }); + let ready = true; + if (predicate) { + ready = Boolean( + await evaluateChromeMcpScript({ + profileName: params.profileName, + targetId: params.targetId, + fn: `async () => ${predicate}`, + }), + ); + } + if (ready && params.url) { + const currentUrl = await evaluateChromeMcpScript({ + profileName: params.profileName, + targetId: params.targetId, + fn: "() => window.location.href", + }); + ready = typeof currentUrl === "string" && matchBrowserUrlPattern(params.url, currentUrl); + } if (ready) { return; } diff --git a/src/browser/routes/agent.existing-session.test.ts b/src/browser/routes/agent.existing-session.test.ts index 077889d16f9..070a0ed41d7 100644 --- a/src/browser/routes/agent.existing-session.test.ts +++ b/src/browser/routes/agent.existing-session.test.ts @@ -195,4 +195,37 @@ describe("existing-session browser routes", () => { }); expect(chromeMcpMocks.evaluateChromeMcpScript).not.toHaveBeenCalled(); }); + + it("supports glob URL waits for existing-session profiles", async () => { + chromeMcpMocks.evaluateChromeMcpScript.mockReset(); + chromeMcpMocks.evaluateChromeMcpScript.mockImplementation( + async ({ fn }: { fn: string }) => + (fn === "() => window.location.href" ? "https://example.com/" : true) as never, + ); + + 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", url: "**/example.com/" }, + }, + response.res, + ); + + expect(response.statusCode).toBe(200); + expect(response.body).toMatchObject({ ok: true, targetId: "7" }); + expect(chromeMcpMocks.evaluateChromeMcpScript).toHaveBeenCalledWith({ + profileName: "chrome-live", + targetId: "7", + fn: "() => window.location.href", + }); + }); }); diff --git a/src/browser/routes/basic.existing-session.test.ts b/src/browser/routes/basic.existing-session.test.ts new file mode 100644 index 00000000000..77366815ca8 --- /dev/null +++ b/src/browser/routes/basic.existing-session.test.ts @@ -0,0 +1,80 @@ +import { describe, expect, it } from "vitest"; +import { BrowserProfileUnavailableError } from "../errors.js"; +import { registerBrowserBasicRoutes } from "./basic.js"; +import type { BrowserResponse, BrowserRouteHandler, BrowserRouteRegistrar } from "./types.js"; + +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 }; +} + +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("basic browser routes", () => { + it("maps existing-session status failures to JSON browser errors", async () => { + const { app, getHandlers } = createApp(); + registerBrowserBasicRoutes(app, { + state: () => ({ + resolved: { + enabled: true, + headless: false, + noSandbox: false, + executablePath: undefined, + }, + profiles: new Map(), + }), + forProfile: () => + ({ + profile: { + name: "chrome-live", + driver: "existing-session", + cdpPort: 18802, + cdpUrl: "http://127.0.0.1:18802", + color: "#00AA00", + attachOnly: true, + }, + isHttpReachable: async () => { + throw new BrowserProfileUnavailableError("attach failed"); + }, + isReachable: async () => true, + }) as never, + } as never); + + const handler = getHandlers.get("/"); + expect(handler).toBeTypeOf("function"); + + const response = createResponse(); + await handler?.({ params: {}, query: { profile: "chrome-live" } }, response.res); + + expect(response.statusCode).toBe(409); + expect(response.body).toMatchObject({ error: "attach failed" }); + }); +}); diff --git a/src/browser/routes/basic.ts b/src/browser/routes/basic.ts index 9991744107d..abb56d5af21 100644 --- a/src/browser/routes/basic.ts +++ b/src/browser/routes/basic.ts @@ -54,50 +54,58 @@ export function registerBrowserBasicRoutes(app: BrowserRouteRegistrar, ctx: Brow return jsonError(res, profileCtx.status, profileCtx.error); } - const [cdpHttp, cdpReady] = await Promise.all([ - profileCtx.isHttpReachable(300), - profileCtx.isReachable(600), - ]); - - const profileState = current.profiles.get(profileCtx.profile.name); - let detectedBrowser: string | null = null; - let detectedExecutablePath: string | null = null; - let detectError: string | null = null; - try { - const detected = resolveBrowserExecutableForPlatform(current.resolved, process.platform); - if (detected) { - detectedBrowser = detected.kind; - detectedExecutablePath = detected.path; - } - } catch (err) { - detectError = String(err); - } + const [cdpHttp, cdpReady] = await Promise.all([ + profileCtx.isHttpReachable(300), + profileCtx.isReachable(600), + ]); - res.json({ - enabled: current.resolved.enabled, - profile: profileCtx.profile.name, - driver: profileCtx.profile.driver, - running: cdpReady, - cdpReady, - cdpHttp, - pid: - profileCtx.profile.driver === "existing-session" - ? getChromeMcpPid(profileCtx.profile.name) - : (profileState?.running?.pid ?? null), - cdpPort: profileCtx.profile.cdpPort, - cdpUrl: profileCtx.profile.cdpUrl, - chosenBrowser: profileState?.running?.exe.kind ?? null, - detectedBrowser, - detectedExecutablePath, - detectError, - userDataDir: profileState?.running?.userDataDir ?? null, - color: profileCtx.profile.color, - headless: current.resolved.headless, - noSandbox: current.resolved.noSandbox, - executablePath: current.resolved.executablePath ?? null, - attachOnly: profileCtx.profile.attachOnly, - }); + const profileState = current.profiles.get(profileCtx.profile.name); + let detectedBrowser: string | null = null; + let detectedExecutablePath: string | null = null; + let detectError: string | null = null; + + try { + const detected = resolveBrowserExecutableForPlatform(current.resolved, process.platform); + if (detected) { + detectedBrowser = detected.kind; + detectedExecutablePath = detected.path; + } + } catch (err) { + detectError = String(err); + } + + res.json({ + enabled: current.resolved.enabled, + profile: profileCtx.profile.name, + driver: profileCtx.profile.driver, + running: cdpReady, + cdpReady, + cdpHttp, + pid: + profileCtx.profile.driver === "existing-session" + ? getChromeMcpPid(profileCtx.profile.name) + : (profileState?.running?.pid ?? null), + cdpPort: profileCtx.profile.cdpPort, + cdpUrl: profileCtx.profile.cdpUrl, + chosenBrowser: profileState?.running?.exe.kind ?? null, + detectedBrowser, + detectedExecutablePath, + detectError, + userDataDir: profileState?.running?.userDataDir ?? null, + color: profileCtx.profile.color, + headless: current.resolved.headless, + noSandbox: current.resolved.noSandbox, + executablePath: current.resolved.executablePath ?? null, + attachOnly: profileCtx.profile.attachOnly, + }); + } catch (err) { + const mapped = toBrowserErrorResponse(err); + if (mapped) { + return jsonError(res, mapped.status, mapped.message); + } + jsonError(res, 500, String(err)); + } }); // Start browser (profile-aware) diff --git a/src/browser/server.agent-contract-snapshot-endpoints.test.ts b/src/browser/server.agent-contract-snapshot-endpoints.test.ts index 7e300fe5aee..837a122becd 100644 --- a/src/browser/server.agent-contract-snapshot-endpoints.test.ts +++ b/src/browser/server.agent-contract-snapshot-endpoints.test.ts @@ -96,10 +96,14 @@ describe("browser control server", () => { headers: { "Content-Type": "application/json" }, body: JSON.stringify({ kind: "click", selector: "button.save" }), }); - expect(clickSelector.status).toBe(400); - expect(((await clickSelector.json()) as { error?: string }).error).toMatch( - /'selector' is not supported/i, - ); + expect(clickSelector.status).toBe(200); + expect(((await clickSelector.json()) as { ok?: boolean }).ok).toBe(true); + expect(pwMocks.clickViaPlaywright).toHaveBeenNthCalledWith(2, { + cdpUrl: state.cdpBaseUrl, + targetId: "abcd1234", + selector: "button.save", + doubleClick: false, + }); const type = await postJson<{ ok: boolean }>(`${base}/act`, { kind: "type", diff --git a/src/browser/url-pattern.ts b/src/browser/url-pattern.ts new file mode 100644 index 00000000000..2ff99657d26 --- /dev/null +++ b/src/browser/url-pattern.ts @@ -0,0 +1,15 @@ +export function matchBrowserUrlPattern(pattern: string, url: string): boolean { + const trimmedPattern = pattern.trim(); + if (!trimmedPattern) { + return false; + } + if (trimmedPattern === url) { + return true; + } + if (trimmedPattern.includes("*")) { + const escaped = trimmedPattern.replace(/[|\\{}()[\]^$+?.]/g, "\\$&"); + const regex = new RegExp(`^${escaped.replace(/\*\*/g, ".*").replace(/\*/g, ".*")}$`); + return regex.test(url); + } + return url.includes(trimmedPattern); +} diff --git a/src/commands/self-hosted-provider-setup.ts b/src/commands/self-hosted-provider-setup.ts index 6a50820ce91..a92b7512d9a 100644 --- a/src/commands/self-hosted-provider-setup.ts +++ b/src/commands/self-hosted-provider-setup.ts @@ -2,6 +2,7 @@ import { upsertAuthProfileWithLock } from "../agents/auth-profiles.js"; import type { ApiKeyCredential, AuthProfileCredential } from "../agents/auth-profiles/types.js"; import type { OpenClawConfig } from "../config/config.js"; import type { + ProviderAuthResult, ProviderAuthMethodNonInteractiveContext, ProviderNonInteractiveApiKeyResult, } from "../plugins/types.js"; @@ -85,7 +86,7 @@ function buildOpenAICompatibleSelfHostedProviderConfig(params: { }; } -export async function promptAndConfigureOpenAICompatibleSelfHostedProvider(params: { +type OpenAICompatibleSelfHostedProviderSetupParams = { cfg: OpenClawConfig; prompter: WizardPrompter; providerId: string; @@ -97,13 +98,34 @@ export async function promptAndConfigureOpenAICompatibleSelfHostedProvider(param reasoning?: boolean; contextWindow?: number; maxTokens?: number; -}): Promise<{ +}; + +type OpenAICompatibleSelfHostedProviderPromptResult = { config: OpenClawConfig; credential: AuthProfileCredential; modelId: string; modelRef: string; profileId: string; -}> { +}; + +function buildSelfHostedProviderAuthResult( + result: OpenAICompatibleSelfHostedProviderPromptResult, +): ProviderAuthResult { + return { + profiles: [ + { + profileId: result.profileId, + credential: result.credential, + }, + ], + configPatch: result.config, + defaultModel: result.modelRef, + }; +} + +export async function promptAndConfigureOpenAICompatibleSelfHostedProvider( + params: OpenAICompatibleSelfHostedProviderSetupParams, +): Promise { const baseUrlRaw = await params.prompter.text({ message: `${params.providerLabel} base URL`, initialValue: params.defaultBaseUrl, @@ -152,6 +174,13 @@ export async function promptAndConfigureOpenAICompatibleSelfHostedProvider(param }; } +export async function promptAndConfigureOpenAICompatibleSelfHostedProviderAuth( + params: OpenAICompatibleSelfHostedProviderSetupParams, +): Promise { + const result = await promptAndConfigureOpenAICompatibleSelfHostedProvider(params); + return buildSelfHostedProviderAuthResult(result); +} + function buildMissingNonInteractiveModelIdMessage(params: { authChoice: string; providerLabel: string; diff --git a/src/commands/status-all/channels.mattermost-token-summary.test.ts b/src/commands/status-all/channels.mattermost-token-summary.test.ts index a797d028d9f..a012a3a3647 100644 --- a/src/commands/status-all/channels.mattermost-token-summary.test.ts +++ b/src/commands/status-all/channels.mattermost-token-summary.test.ts @@ -37,139 +37,94 @@ function makeMattermostPlugin(): ChannelPlugin { }; } -function makeSlackPlugin(params?: { botToken?: string; appToken?: string }): ChannelPlugin { - return { - id: "slack", - meta: { - id: "slack", - label: "Slack", - selectionLabel: "Slack", - docsPath: "/channels/slack", - blurb: "test", - }, - capabilities: { chatTypes: ["direct"] }, - config: { - listAccountIds: () => ["primary"], - defaultAccountId: () => "primary", - inspectAccount: () => ({ - name: "Primary", - enabled: true, - botToken: params?.botToken ?? "bot-token", - appToken: params?.appToken ?? "app-token", - }), - resolveAccount: () => ({ - name: "Primary", - enabled: true, - botToken: params?.botToken ?? "bot-token", - appToken: params?.appToken ?? "app-token", - }), - isConfigured: () => true, - isEnabled: () => true, - }, - actions: { - listActions: () => ["send"], - }, - }; -} +type TestTable = Awaited>; -function makeUnavailableSlackPlugin(): ChannelPlugin { - return { - id: "slack", - meta: { - id: "slack", - label: "Slack", - selectionLabel: "Slack", - docsPath: "/channels/slack", - blurb: "test", - }, - capabilities: { chatTypes: ["direct"] }, - config: { - listAccountIds: () => ["primary"], - defaultAccountId: () => "primary", - inspectAccount: () => ({ - name: "Primary", - enabled: true, - configured: true, - botToken: "", - appToken: "", - botTokenSource: "config", - appTokenSource: "config", - botTokenStatus: "configured_unavailable", - appTokenStatus: "configured_unavailable", - }), - resolveAccount: () => ({ - name: "Primary", - enabled: true, - configured: true, - botToken: "", - appToken: "", - botTokenSource: "config", - appTokenSource: "config", - botTokenStatus: "configured_unavailable", - appTokenStatus: "configured_unavailable", - }), - isConfigured: () => true, - isEnabled: () => true, - }, - actions: { - listActions: () => ["send"], - }, - }; -} - -function makeSourceAwareUnavailablePlugin(): ChannelPlugin { +function makeSlackDirectPlugin(config: ChannelPlugin["config"]): ChannelPlugin { return makeDirectPlugin({ id: "slack", label: "Slack", docsPath: "/channels/slack", - config: { - listAccountIds: () => ["primary"], - defaultAccountId: () => "primary", - inspectAccount: (cfg) => - (cfg as { marker?: string }).marker === "source" - ? { - name: "Primary", - enabled: true, - configured: true, - botToken: "", - appToken: "", - botTokenSource: "config", - appTokenSource: "config", - botTokenStatus: "configured_unavailable", - appTokenStatus: "configured_unavailable", - } - : { - name: "Primary", - enabled: true, - configured: false, - botToken: "", - appToken: "", - botTokenSource: "none", - appTokenSource: "none", - }, - resolveAccount: () => ({ - name: "Primary", - enabled: true, - botToken: "", - appToken: "", - }), - isConfigured: (account) => Boolean((account as { configured?: boolean }).configured), - isEnabled: () => true, - }, + config, + }); +} + +function createSlackTokenAccount(params?: { botToken?: string; appToken?: string }) { + return { + name: "Primary", + enabled: true, + botToken: params?.botToken ?? "bot-token", + appToken: params?.appToken ?? "app-token", + }; +} + +function createUnavailableSlackTokenAccount() { + return { + name: "Primary", + enabled: true, + configured: true, + botToken: "", + appToken: "", + botTokenSource: "config", + appTokenSource: "config", + botTokenStatus: "configured_unavailable", + appTokenStatus: "configured_unavailable", + }; +} + +function makeSlackPlugin(params?: { botToken?: string; appToken?: string }): ChannelPlugin { + return makeSlackDirectPlugin({ + listAccountIds: () => ["primary"], + defaultAccountId: () => "primary", + inspectAccount: () => createSlackTokenAccount(params), + resolveAccount: () => createSlackTokenAccount(params), + isConfigured: () => true, + isEnabled: () => true, + }); +} + +function makeUnavailableSlackPlugin(): ChannelPlugin { + return makeSlackDirectPlugin({ + listAccountIds: () => ["primary"], + defaultAccountId: () => "primary", + inspectAccount: () => createUnavailableSlackTokenAccount(), + resolveAccount: () => createUnavailableSlackTokenAccount(), + isConfigured: () => true, + isEnabled: () => true, + }); +} + +function makeSourceAwareUnavailablePlugin(): ChannelPlugin { + return makeSlackDirectPlugin({ + listAccountIds: () => ["primary"], + defaultAccountId: () => "primary", + inspectAccount: (cfg) => + (cfg as { marker?: string }).marker === "source" + ? createUnavailableSlackTokenAccount() + : { + name: "Primary", + enabled: true, + configured: false, + botToken: "", + appToken: "", + botTokenSource: "none", + appTokenSource: "none", + }, + resolveAccount: () => ({ + name: "Primary", + enabled: true, + botToken: "", + appToken: "", + }), + isConfigured: (account) => Boolean((account as { configured?: boolean }).configured), + isEnabled: () => true, }); } function makeSourceUnavailableResolvedAvailablePlugin(): ChannelPlugin { - return { + return makeDirectPlugin({ id: "discord", - meta: { - id: "discord", - label: "Discord", - selectionLabel: "Discord", - docsPath: "/channels/discord", - blurb: "test", - }, - capabilities: { chatTypes: ["direct"] }, + label: "Discord", + docsPath: "/channels/discord", config: { listAccountIds: () => ["primary"], defaultAccountId: () => "primary", @@ -199,10 +154,7 @@ function makeSourceUnavailableResolvedAvailablePlugin(): ChannelPlugin { isConfigured: (account) => Boolean((account as { configured?: boolean }).configured), isEnabled: () => true, }, - actions: { - listActions: () => ["send"], - }, - }; + }); } function makeHttpSlackUnavailablePlugin(): ChannelPlugin { @@ -263,64 +215,76 @@ function makeTokenPlugin(): ChannelPlugin { }); } +async function buildTestTable( + plugins: ChannelPlugin[], + params?: { cfg?: Record; sourceConfig?: Record }, +) { + vi.mocked(listChannelPlugins).mockReturnValue(plugins); + return await buildChannelsTable((params?.cfg ?? { channels: {} }) as never, { + showSecrets: false, + sourceConfig: params?.sourceConfig as never, + }); +} + +function expectTableRow( + table: TestTable, + params: { id: string; state: string; detailContains?: string; detailEquals?: string }, +) { + const row = table.rows.find((entry) => entry.id === params.id); + expect(row).toBeDefined(); + expect(row?.state).toBe(params.state); + if (params.detailContains) { + expect(row?.detail).toContain(params.detailContains); + } + if (params.detailEquals) { + expect(row?.detail).toBe(params.detailEquals); + } + return row; +} + +function expectTableDetailRows( + table: TestTable, + title: string, + rows: Array>, +) { + const detail = table.details.find((entry) => entry.title === title); + expect(detail).toBeDefined(); + expect(detail?.rows).toEqual(rows); +} + describe("buildChannelsTable - mattermost token summary", () => { it("does not require appToken for mattermost accounts", async () => { - vi.mocked(listChannelPlugins).mockReturnValue([makeMattermostPlugin()]); - - const table = await buildChannelsTable({ channels: {} } as never, { - showSecrets: false, - }); - - const mattermostRow = table.rows.find((row) => row.id === "mattermost"); - expect(mattermostRow).toBeDefined(); - expect(mattermostRow?.state).toBe("ok"); + const table = await buildTestTable([makeMattermostPlugin()]); + const mattermostRow = expectTableRow(table, { id: "mattermost", state: "ok" }); expect(mattermostRow?.detail).not.toContain("need bot+app"); }); it("keeps bot+app requirement when both fields exist", async () => { - vi.mocked(listChannelPlugins).mockReturnValue([ - makeSlackPlugin({ botToken: "bot-token", appToken: "" }), - ]); - - const table = await buildChannelsTable({ channels: {} } as never, { - showSecrets: false, - }); - - const slackRow = table.rows.find((row) => row.id === "slack"); - expect(slackRow).toBeDefined(); - expect(slackRow?.state).toBe("warn"); - expect(slackRow?.detail).toContain("need bot+app"); + const table = await buildTestTable([makeSlackPlugin({ botToken: "bot-token", appToken: "" })]); + expectTableRow(table, { id: "slack", state: "warn", detailContains: "need bot+app" }); }); it("reports configured-but-unavailable Slack credentials as warn", async () => { - vi.mocked(listChannelPlugins).mockReturnValue([makeUnavailableSlackPlugin()]); - - const table = await buildChannelsTable({ channels: {} } as never, { - showSecrets: false, + const table = await buildTestTable([makeUnavailableSlackPlugin()]); + expectTableRow(table, { + id: "slack", + state: "warn", + detailContains: "unavailable in this command path", }); - - const slackRow = table.rows.find((row) => row.id === "slack"); - expect(slackRow).toBeDefined(); - expect(slackRow?.state).toBe("warn"); - expect(slackRow?.detail).toContain("unavailable in this command path"); }); it("preserves unavailable credential state from the source config snapshot", async () => { - vi.mocked(listChannelPlugins).mockReturnValue([makeSourceAwareUnavailablePlugin()]); - - const table = await buildChannelsTable({ marker: "resolved", channels: {} } as never, { - showSecrets: false, - sourceConfig: { marker: "source", channels: {} } as never, + const table = await buildTestTable([makeSourceAwareUnavailablePlugin()], { + cfg: { marker: "resolved", channels: {} }, + sourceConfig: { marker: "source", channels: {} }, }); - const slackRow = table.rows.find((row) => row.id === "slack"); - expect(slackRow).toBeDefined(); - expect(slackRow?.state).toBe("warn"); - expect(slackRow?.detail).toContain("unavailable in this command path"); - - const slackDetails = table.details.find((detail) => detail.title === "Slack accounts"); - expect(slackDetails).toBeDefined(); - expect(slackDetails?.rows).toEqual([ + expectTableRow(table, { + id: "slack", + state: "warn", + detailContains: "unavailable in this command path", + }); + expectTableDetailRows(table, "Slack accounts", [ { Account: "primary (Primary)", Notes: "bot:config · app:config · secret unavailable in this command path", @@ -330,21 +294,13 @@ describe("buildChannelsTable - mattermost token summary", () => { }); it("treats status-only available credentials as resolved", async () => { - vi.mocked(listChannelPlugins).mockReturnValue([makeSourceUnavailableResolvedAvailablePlugin()]); - - const table = await buildChannelsTable({ marker: "resolved", channels: {} } as never, { - showSecrets: false, - sourceConfig: { marker: "source", channels: {} } as never, + const table = await buildTestTable([makeSourceUnavailableResolvedAvailablePlugin()], { + cfg: { marker: "resolved", channels: {} }, + sourceConfig: { marker: "source", channels: {} }, }); - const discordRow = table.rows.find((row) => row.id === "discord"); - expect(discordRow).toBeDefined(); - expect(discordRow?.state).toBe("ok"); - expect(discordRow?.detail).toBe("configured"); - - const discordDetails = table.details.find((detail) => detail.title === "Discord accounts"); - expect(discordDetails).toBeDefined(); - expect(discordDetails?.rows).toEqual([ + expectTableRow(table, { id: "discord", state: "ok", detailEquals: "configured" }); + expectTableDetailRows(table, "Discord accounts", [ { Account: "primary (Primary)", Notes: "token:config", @@ -354,20 +310,13 @@ describe("buildChannelsTable - mattermost token summary", () => { }); it("treats Slack HTTP signing-secret availability as required config", async () => { - vi.mocked(listChannelPlugins).mockReturnValue([makeHttpSlackUnavailablePlugin()]); - - const table = await buildChannelsTable({ channels: {} } as never, { - showSecrets: false, + const table = await buildTestTable([makeHttpSlackUnavailablePlugin()]); + expectTableRow(table, { + id: "slack", + state: "warn", + detailContains: "configured http credentials unavailable", }); - - const slackRow = table.rows.find((row) => row.id === "slack"); - expect(slackRow).toBeDefined(); - expect(slackRow?.state).toBe("warn"); - expect(slackRow?.detail).toContain("configured http credentials unavailable"); - - const slackDetails = table.details.find((detail) => detail.title === "Slack accounts"); - expect(slackDetails).toBeDefined(); - expect(slackDetails?.rows).toEqual([ + expectTableDetailRows(table, "Slack accounts", [ { Account: "primary (Primary)", Notes: "bot:config · signing:config · secret unavailable in this command path", @@ -377,15 +326,7 @@ describe("buildChannelsTable - mattermost token summary", () => { }); it("still reports single-token channels as ok", async () => { - vi.mocked(listChannelPlugins).mockReturnValue([makeTokenPlugin()]); - - const table = await buildChannelsTable({ channels: {} } as never, { - showSecrets: false, - }); - - const tokenRow = table.rows.find((row) => row.id === "token-only"); - expect(tokenRow).toBeDefined(); - expect(tokenRow?.state).toBe("ok"); - expect(tokenRow?.detail).toContain("token"); + const table = await buildTestTable([makeTokenPlugin()]); + expectTableRow(table, { id: "token-only", state: "ok", detailContains: "token" }); }); }); diff --git a/src/config/sessions/targets.test.ts b/src/config/sessions/targets.test.ts index 720cc3e892e..43674233a3a 100644 --- a/src/config/sessions/targets.test.ts +++ b/src/config/sessions/targets.test.ts @@ -40,6 +40,14 @@ function createCustomRootCfg(customRoot: string, defaultAgentId = "ops"): OpenCl }; } +async function resolveTargetsForCustomRoot(home: string, agentIds: string[]) { + const customRoot = path.join(home, "custom-state"); + const storePaths = await createAgentSessionStores(customRoot, agentIds); + const cfg = createCustomRootCfg(customRoot); + const targets = await resolveAllAgentSessionStoreTargets(cfg, { env: process.env }); + return { storePaths, targets }; +} + function expectTargetsToContainStores( targets: Array<{ agentId: string; storePath: string }>, stores: Record, @@ -152,11 +160,7 @@ describe("resolveAllAgentSessionStoreTargets", () => { it("discovers retired agent stores under a configured custom session root", async () => { await withTempHome(async (home) => { - const customRoot = path.join(home, "custom-state"); - const storePaths = await createAgentSessionStores(customRoot, ["ops", "retired"]); - const cfg = createCustomRootCfg(customRoot); - - const targets = await resolveAllAgentSessionStoreTargets(cfg, { env: process.env }); + const { storePaths, targets } = await resolveTargetsForCustomRoot(home, ["ops", "retired"]); expectTargetsToContainStores(targets, storePaths); expect(targets.filter((target) => target.storePath === storePaths.ops)).toHaveLength(1); @@ -165,11 +169,10 @@ describe("resolveAllAgentSessionStoreTargets", () => { it("keeps the actual on-disk store path for discovered retired agents", async () => { await withTempHome(async (home) => { - const customRoot = path.join(home, "custom-state"); - const storePaths = await createAgentSessionStores(customRoot, ["ops", "Retired Agent"]); - const cfg = createCustomRootCfg(customRoot); - - const targets = await resolveAllAgentSessionStoreTargets(cfg, { env: process.env }); + const { storePaths, targets } = await resolveTargetsForCustomRoot(home, [ + "ops", + "Retired Agent", + ]); expect(targets).toEqual( expect.arrayContaining([ diff --git a/src/discord/monitor/allow-list.ts b/src/discord/monitor/allow-list.ts index 583d4fa7cd2..ef29f1fc706 100644 --- a/src/discord/monitor/allow-list.ts +++ b/src/discord/monitor/allow-list.ts @@ -19,34 +19,7 @@ export type DiscordAllowListMatch = AllowlistMatch<"wildcard" | "id" | "name" | const DISCORD_OWNER_ALLOWLIST_PREFIXES = ["discord:", "user:", "pk:"]; -export type DiscordGuildEntryResolved = { - id?: string; - slug?: string; - requireMention?: boolean; - ignoreOtherMentions?: boolean; - reactionNotifications?: "off" | "own" | "all" | "allowlist"; - users?: string[]; - roles?: string[]; - channels?: Record< - string, - { - allow?: boolean; - requireMention?: boolean; - ignoreOtherMentions?: boolean; - skills?: string[]; - enabled?: boolean; - users?: string[]; - roles?: string[]; - systemPrompt?: string; - includeThreadStarter?: boolean; - autoThread?: boolean; - autoArchiveDuration?: "60" | "1440" | "4320" | "10080" | 60 | 1440 | 4320 | 10080; - } - >; -}; - -export type DiscordChannelConfigResolved = { - allowed: boolean; +type DiscordChannelOverrideConfig = { requireMention?: boolean; ignoreOtherMentions?: boolean; skills?: string[]; @@ -57,6 +30,21 @@ export type DiscordChannelConfigResolved = { includeThreadStarter?: boolean; autoThread?: boolean; autoArchiveDuration?: "60" | "1440" | "4320" | "10080" | 60 | 1440 | 4320 | 10080; +}; + +export type DiscordGuildEntryResolved = { + id?: string; + slug?: string; + requireMention?: boolean; + ignoreOtherMentions?: boolean; + reactionNotifications?: "off" | "own" | "all" | "allowlist"; + users?: string[]; + roles?: string[]; + channels?: Record; +}; + +export type DiscordChannelConfigResolved = DiscordChannelOverrideConfig & { + allowed: boolean; matchKey?: string; matchSource?: ChannelMatchSource; }; diff --git a/src/discord/monitor/exec-approvals.test.ts b/src/discord/monitor/exec-approvals.test.ts index 8f9430393a2..c7cb72b82ec 100644 --- a/src/discord/monitor/exec-approvals.test.ts +++ b/src/discord/monitor/exec-approvals.test.ts @@ -116,6 +116,62 @@ function createHandler(config: DiscordExecApprovalConfig, accountId = "default") }); } +function mockSuccessfulDmDelivery(params?: { + noteChannelId?: string; + expectedNoteText?: string; + throwOnUnexpectedRoute?: boolean; +}) { + mockRestPost.mockImplementation( + async (route: string, requestParams?: { body?: { content?: string } }) => { + if (params?.noteChannelId && route === Routes.channelMessages(params.noteChannelId)) { + if (params.expectedNoteText) { + expect(requestParams?.body?.content).toContain(params.expectedNoteText); + } + return { id: "note-1", channel_id: params.noteChannelId }; + } + if (route === Routes.userChannels()) { + return { id: "dm-1" }; + } + if (route === Routes.channelMessages("dm-1")) { + return { id: "msg-1", channel_id: "dm-1" }; + } + if (params?.throwOnUnexpectedRoute) { + throw new Error(`unexpected route: ${route}`); + } + return { id: "msg-unknown" }; + }, + ); +} + +async function expectGatewayAuthStart(params: { + handler: DiscordExecApprovalHandler; + expectedUrl: string; + expectedSource: "cli" | "env"; + expectedToken?: string; + expectedPassword?: string; +}) { + await params.handler.start(); + + expect(mockResolveGatewayConnectionAuth).toHaveBeenCalledWith( + expect.objectContaining({ + env: process.env, + urlOverride: params.expectedUrl, + urlOverrideSource: params.expectedSource, + }), + ); + + const expectedClientParams: Record = { + url: params.expectedUrl, + }; + if (params.expectedToken !== undefined) { + expectedClientParams.token = params.expectedToken; + } + if (params.expectedPassword !== undefined) { + expectedClientParams.password = params.expectedPassword; + } + expect(mockGatewayClientCtor).toHaveBeenCalledWith(expect.objectContaining(expectedClientParams)); +} + type ExecApprovalHandlerInternals = { pending: Map< string, @@ -772,15 +828,7 @@ describe("DiscordExecApprovalHandler delivery routing", () => { }); const internals = getHandlerInternals(handler); - mockRestPost.mockImplementation(async (route: string) => { - if (route === Routes.userChannels()) { - return { id: "dm-1" }; - } - if (route === Routes.channelMessages("dm-1")) { - return { id: "msg-1", channel_id: "dm-1" }; - } - return { id: "msg-unknown" }; - }); + mockSuccessfulDmDelivery(); const request = createRequest({ sessionKey: "agent:main:discord:dm:123" }); await internals.handleApprovalRequested(request); @@ -809,21 +857,11 @@ describe("DiscordExecApprovalHandler delivery routing", () => { }); const internals = getHandlerInternals(handler); - mockRestPost.mockImplementation( - async (route: string, params?: { body?: { content?: string } }) => { - if (route === Routes.channelMessages("999888777")) { - expect(params?.body?.content).toContain("I sent the allowed approvers DMs"); - return { id: "note-1", channel_id: "999888777" }; - } - if (route === Routes.userChannels()) { - return { id: "dm-1" }; - } - if (route === Routes.channelMessages("dm-1")) { - return { id: "msg-1", channel_id: "dm-1" }; - } - throw new Error(`unexpected route: ${route}`); - }, - ); + mockSuccessfulDmDelivery({ + noteChannelId: "999888777", + expectedNoteText: "I sent the allowed approvers DMs", + throwOnUnexpectedRoute: true, + }); await internals.handleApprovalRequested(createRequest()); @@ -853,15 +891,7 @@ describe("DiscordExecApprovalHandler delivery routing", () => { }); const internals = getHandlerInternals(handler); - mockRestPost.mockImplementation(async (route: string) => { - if (route === Routes.userChannels()) { - return { id: "dm-1" }; - } - if (route === Routes.channelMessages("dm-1")) { - return { id: "msg-1", channel_id: "dm-1" }; - } - throw new Error(`unexpected route: ${route}`); - }); + mockSuccessfulDmDelivery({ throwOnUnexpectedRoute: true }); await internals.handleApprovalRequested( createRequest({ sessionKey: "agent:main:discord:dm:123" }), @@ -890,22 +920,13 @@ describe("DiscordExecApprovalHandler gateway auth resolution", () => { cfg: { session: { store: STORE_PATH } }, }); - await handler.start(); - - expect(mockResolveGatewayConnectionAuth).toHaveBeenCalledWith( - expect.objectContaining({ - env: process.env, - urlOverride: "wss://override.example/ws", - urlOverrideSource: "cli", - }), - ); - expect(mockGatewayClientCtor).toHaveBeenCalledWith( - expect.objectContaining({ - url: "wss://override.example/ws", - token: "resolved-token", - password: "resolved-password", // pragma: allowlist secret - }), - ); + await expectGatewayAuthStart({ + handler, + expectedUrl: "wss://override.example/ws", + expectedSource: "cli", + expectedToken: "resolved-token", + expectedPassword: "resolved-password", // pragma: allowlist secret + }); await handler.stop(); }); @@ -921,20 +942,11 @@ describe("DiscordExecApprovalHandler gateway auth resolution", () => { cfg: { session: { store: STORE_PATH } }, }); - await handler.start(); - - expect(mockResolveGatewayConnectionAuth).toHaveBeenCalledWith( - expect.objectContaining({ - env: process.env, - urlOverride: "wss://gateway-from-env.example/ws", - urlOverrideSource: "env", - }), - ); - expect(mockGatewayClientCtor).toHaveBeenCalledWith( - expect.objectContaining({ - url: "wss://gateway-from-env.example/ws", - }), - ); + await expectGatewayAuthStart({ + handler, + expectedUrl: "wss://gateway-from-env.example/ws", + expectedSource: "env", + }); await handler.stop(); } finally { diff --git a/src/discord/monitor/exec-approvals.ts b/src/discord/monitor/exec-approvals.ts index 87dc0c9a07d..8dd3156e991 100644 --- a/src/discord/monitor/exec-approvals.ts +++ b/src/discord/monitor/exec-approvals.ts @@ -252,17 +252,30 @@ function formatOptionalCommandPreview( return formatCommandPreview(commandText, maxChars); } +function resolveExecApprovalPreviews( + request: ExecApprovalRequest["request"], + maxChars: number, + secondaryMaxChars: number, +): { commandPreview: string; commandSecondaryPreview: string | null } { + const { commandText, commandPreview: secondaryPreview } = + resolveExecApprovalCommandDisplay(request); + return { + commandPreview: formatCommandPreview(commandText, maxChars), + commandSecondaryPreview: formatOptionalCommandPreview(secondaryPreview, secondaryMaxChars), + }; +} + function createExecApprovalRequestContainer(params: { request: ExecApprovalRequest; cfg: OpenClawConfig; accountId: string; actionRow?: Row