diff --git a/extensions/acpx/src/runtime-internals/process.test.ts b/extensions/acpx/src/runtime-internals/process.test.ts index ba6ad923d3b..ef0492308ae 100644 --- a/extensions/acpx/src/runtime-internals/process.test.ts +++ b/extensions/acpx/src/runtime-internals/process.test.ts @@ -254,6 +254,44 @@ describe("waitForExit", () => { }); describe("spawnAndCollect", () => { + type SpawnedEnvSnapshot = { + openai?: string; + github?: string; + hf?: string; + openclaw?: string; + shell?: string; + }; + + function stubProviderAuthEnv(env: Record) { + for (const [key, value] of Object.entries(env)) { + vi.stubEnv(key, value); + } + } + + async function collectSpawnedEnvSnapshot(options?: { + stripProviderAuthEnvVars?: boolean; + openAiEnvKey?: string; + githubEnvKey?: string; + hfEnvKey?: string; + }): Promise { + const openAiEnvKey = options?.openAiEnvKey ?? "OPENAI_API_KEY"; + const githubEnvKey = options?.githubEnvKey ?? "GITHUB_TOKEN"; + const hfEnvKey = options?.hfEnvKey ?? "HF_TOKEN"; + const result = await spawnAndCollect({ + command: process.execPath, + args: [ + "-e", + `process.stdout.write(JSON.stringify({openai:process.env.${openAiEnvKey},github:process.env.${githubEnvKey},hf:process.env.${hfEnvKey},openclaw:process.env.OPENCLAW_API_KEY,shell:process.env.OPENCLAW_SHELL}))`, + ], + cwd: process.cwd(), + stripProviderAuthEnvVars: options?.stripProviderAuthEnvVars, + }); + + expect(result.code).toBe(0); + expect(result.error).toBeNull(); + return JSON.parse(result.stdout) as SpawnedEnvSnapshot; + } + it("returns abort error immediately when signal is already aborted", async () => { const controller = new AbortController(); controller.abort(); @@ -292,31 +330,15 @@ describe("spawnAndCollect", () => { }); it("strips shared provider auth env vars from spawned acpx children", async () => { - vi.stubEnv("OPENAI_API_KEY", "openai-secret"); - vi.stubEnv("GITHUB_TOKEN", "gh-secret"); - vi.stubEnv("HF_TOKEN", "hf-secret"); - vi.stubEnv("OPENCLAW_API_KEY", "keep-me"); - - const result = await spawnAndCollect({ - command: process.execPath, - args: [ - "-e", - "process.stdout.write(JSON.stringify({openai:process.env.OPENAI_API_KEY,github:process.env.GITHUB_TOKEN,hf:process.env.HF_TOKEN,openclaw:process.env.OPENCLAW_API_KEY,shell:process.env.OPENCLAW_SHELL}))", - ], - cwd: process.cwd(), + stubProviderAuthEnv({ + OPENAI_API_KEY: "openai-secret", + GITHUB_TOKEN: "gh-secret", + HF_TOKEN: "hf-secret", + OPENCLAW_API_KEY: "keep-me", + }); + const parsed = await collectSpawnedEnvSnapshot({ stripProviderAuthEnvVars: true, }); - - expect(result.code).toBe(0); - expect(result.error).toBeNull(); - - const parsed = JSON.parse(result.stdout) as { - openai?: string; - github?: string; - hf?: string; - openclaw?: string; - shell?: string; - }; expect(parsed.openai).toBeUndefined(); expect(parsed.github).toBeUndefined(); expect(parsed.hf).toBeUndefined(); @@ -325,29 +347,16 @@ describe("spawnAndCollect", () => { }); it("strips provider auth env vars case-insensitively", async () => { - vi.stubEnv("OpenAI_Api_Key", "openai-secret"); - vi.stubEnv("Github_Token", "gh-secret"); - vi.stubEnv("OPENCLAW_API_KEY", "keep-me"); - - const result = await spawnAndCollect({ - command: process.execPath, - args: [ - "-e", - "process.stdout.write(JSON.stringify({openai:process.env.OpenAI_Api_Key,github:process.env.Github_Token,openclaw:process.env.OPENCLAW_API_KEY,shell:process.env.OPENCLAW_SHELL}))", - ], - cwd: process.cwd(), - stripProviderAuthEnvVars: true, + stubProviderAuthEnv({ + OpenAI_Api_Key: "openai-secret", + Github_Token: "gh-secret", + OPENCLAW_API_KEY: "keep-me", + }); + const parsed = await collectSpawnedEnvSnapshot({ + stripProviderAuthEnvVars: true, + openAiEnvKey: "OpenAI_Api_Key", + githubEnvKey: "Github_Token", }); - - expect(result.code).toBe(0); - expect(result.error).toBeNull(); - - const parsed = JSON.parse(result.stdout) as { - openai?: string; - github?: string; - openclaw?: string; - shell?: string; - }; expect(parsed.openai).toBeUndefined(); expect(parsed.github).toBeUndefined(); expect(parsed.openclaw).toBe("keep-me"); @@ -355,30 +364,13 @@ describe("spawnAndCollect", () => { }); it("preserves provider auth env vars for explicit custom commands by default", async () => { - vi.stubEnv("OPENAI_API_KEY", "openai-secret"); - vi.stubEnv("GITHUB_TOKEN", "gh-secret"); - vi.stubEnv("HF_TOKEN", "hf-secret"); - vi.stubEnv("OPENCLAW_API_KEY", "keep-me"); - - const result = await spawnAndCollect({ - command: process.execPath, - args: [ - "-e", - "process.stdout.write(JSON.stringify({openai:process.env.OPENAI_API_KEY,github:process.env.GITHUB_TOKEN,hf:process.env.HF_TOKEN,openclaw:process.env.OPENCLAW_API_KEY,shell:process.env.OPENCLAW_SHELL}))", - ], - cwd: process.cwd(), + stubProviderAuthEnv({ + OPENAI_API_KEY: "openai-secret", + GITHUB_TOKEN: "gh-secret", + HF_TOKEN: "hf-secret", + OPENCLAW_API_KEY: "keep-me", }); - - expect(result.code).toBe(0); - expect(result.error).toBeNull(); - - const parsed = JSON.parse(result.stdout) as { - openai?: string; - github?: string; - hf?: string; - openclaw?: string; - shell?: string; - }; + const parsed = await collectSpawnedEnvSnapshot(); expect(parsed.openai).toBe("openai-secret"); expect(parsed.github).toBe("gh-secret"); expect(parsed.hf).toBe("hf-secret"); diff --git a/extensions/bluebubbles/src/attachments.test.ts b/extensions/bluebubbles/src/attachments.test.ts index 8ef94cf08ae..704b907eb8b 100644 --- a/extensions/bluebubbles/src/attachments.test.ts +++ b/extensions/bluebubbles/src/attachments.test.ts @@ -82,6 +82,15 @@ describe("downloadBlueBubblesAttachment", () => { ).rejects.toThrow("too large"); } + function mockSuccessfulAttachmentDownload(buffer = new Uint8Array([1])) { + mockFetch.mockResolvedValueOnce({ + ok: true, + headers: new Headers(), + arrayBuffer: () => Promise.resolve(buffer.buffer), + }); + return buffer; + } + it("throws when guid is missing", async () => { const attachment: BlueBubblesAttachment = {}; await expect( @@ -159,12 +168,7 @@ describe("downloadBlueBubblesAttachment", () => { }); it("encodes guid in URL", async () => { - const mockBuffer = new Uint8Array([1]); - mockFetch.mockResolvedValueOnce({ - ok: true, - headers: new Headers(), - arrayBuffer: () => Promise.resolve(mockBuffer.buffer), - }); + mockSuccessfulAttachmentDownload(); const attachment: BlueBubblesAttachment = { guid: "att/with/special chars" }; await downloadBlueBubblesAttachment(attachment, { @@ -244,12 +248,7 @@ describe("downloadBlueBubblesAttachment", () => { }); it("resolves credentials from config when opts not provided", async () => { - const mockBuffer = new Uint8Array([1]); - mockFetch.mockResolvedValueOnce({ - ok: true, - headers: new Headers(), - arrayBuffer: () => Promise.resolve(mockBuffer.buffer), - }); + mockSuccessfulAttachmentDownload(); const attachment: BlueBubblesAttachment = { guid: "att-config" }; const result = await downloadBlueBubblesAttachment(attachment, { @@ -270,12 +269,7 @@ describe("downloadBlueBubblesAttachment", () => { }); it("passes ssrfPolicy with allowPrivateNetwork when config enables it", async () => { - const mockBuffer = new Uint8Array([1]); - mockFetch.mockResolvedValueOnce({ - ok: true, - headers: new Headers(), - arrayBuffer: () => Promise.resolve(mockBuffer.buffer), - }); + mockSuccessfulAttachmentDownload(); const attachment: BlueBubblesAttachment = { guid: "att-ssrf" }; await downloadBlueBubblesAttachment(attachment, { @@ -295,12 +289,7 @@ describe("downloadBlueBubblesAttachment", () => { }); it("auto-allowlists serverUrl hostname when allowPrivateNetwork is not set", async () => { - const mockBuffer = new Uint8Array([1]); - mockFetch.mockResolvedValueOnce({ - ok: true, - headers: new Headers(), - arrayBuffer: () => Promise.resolve(mockBuffer.buffer), - }); + mockSuccessfulAttachmentDownload(); const attachment: BlueBubblesAttachment = { guid: "att-no-ssrf" }; await downloadBlueBubblesAttachment(attachment, { @@ -313,12 +302,7 @@ describe("downloadBlueBubblesAttachment", () => { }); it("auto-allowlists private IP serverUrl hostname when allowPrivateNetwork is not set", async () => { - const mockBuffer = new Uint8Array([1]); - mockFetch.mockResolvedValueOnce({ - ok: true, - headers: new Headers(), - arrayBuffer: () => Promise.resolve(mockBuffer.buffer), - }); + mockSuccessfulAttachmentDownload(); const attachment: BlueBubblesAttachment = { guid: "att-private-ip" }; await downloadBlueBubblesAttachment(attachment, { @@ -352,6 +336,14 @@ describe("sendBlueBubblesAttachment", () => { return Buffer.from(body).toString("utf8"); } + function expectVoiceAttachmentBody() { + const body = mockFetch.mock.calls[0][1]?.body as Uint8Array; + const bodyText = decodeBody(body); + expect(bodyText).toContain('name="isAudioMessage"'); + expect(bodyText).toContain("true"); + return bodyText; + } + it("marks voice memos when asVoice is true and mp3 is provided", async () => { mockFetch.mockResolvedValueOnce({ ok: true, @@ -367,10 +359,7 @@ describe("sendBlueBubblesAttachment", () => { opts: { serverUrl: "http://localhost:1234", password: "test" }, }); - const body = mockFetch.mock.calls[0][1]?.body as Uint8Array; - const bodyText = decodeBody(body); - expect(bodyText).toContain('name="isAudioMessage"'); - expect(bodyText).toContain("true"); + const bodyText = expectVoiceAttachmentBody(); expect(bodyText).toContain('filename="voice.mp3"'); }); @@ -389,8 +378,7 @@ describe("sendBlueBubblesAttachment", () => { opts: { serverUrl: "http://localhost:1234", password: "test" }, }); - const body = mockFetch.mock.calls[0][1]?.body as Uint8Array; - const bodyText = decodeBody(body); + const bodyText = expectVoiceAttachmentBody(); expect(bodyText).toContain('filename="voice.mp3"'); expect(bodyText).toContain('name="voice.mp3"'); }); diff --git a/extensions/bluebubbles/src/chat.ts b/extensions/bluebubbles/src/chat.ts index b63f09272f2..1670f276ba7 100644 --- a/extensions/bluebubbles/src/chat.ts +++ b/extensions/bluebubbles/src/chat.ts @@ -26,6 +26,14 @@ 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; } @@ -55,12 +63,7 @@ async function sendBlueBubblesChatEndpointRequest(params: { { method: params.method }, params.opts.timeoutMs, ); - if (!res.ok) { - const errorText = await res.text().catch(() => ""); - throw new Error( - `BlueBubbles ${params.action} failed (${res.status}): ${errorText || "unknown"}`, - ); - } + await assertBlueBubblesActionOk(res, params.action); } async function sendPrivateApiJsonRequest(params: { @@ -86,12 +89,7 @@ async function sendPrivateApiJsonRequest(params: { } const res = await blueBubblesFetchWithTimeout(url, request, params.opts.timeoutMs); - if (!res.ok) { - const errorText = await res.text().catch(() => ""); - throw new Error( - `BlueBubbles ${params.action} failed (${res.status}): ${errorText || "unknown"}`, - ); - } + await assertBlueBubblesActionOk(res, params.action); } export async function markBlueBubblesChatRead( diff --git a/extensions/bluebubbles/src/monitor-normalize.ts b/extensions/bluebubbles/src/monitor-normalize.ts index 83454602d4c..085bd8923e1 100644 --- a/extensions/bluebubbles/src/monitor-normalize.ts +++ b/extensions/bluebubbles/src/monitor-normalize.ts @@ -582,6 +582,29 @@ export function parseTapbackText(params: { return null; } + const parseLeadingReactionAction = ( + prefix: "reacted" | "removed", + defaultAction: "added" | "removed", + ) => { + if (!lower.startsWith(prefix)) { + return null; + } + const emoji = extractFirstEmoji(trimmed) ?? params.emojiHint; + if (!emoji) { + return null; + } + const quotedText = extractQuotedTapbackText(trimmed); + if (params.requireQuoted && !quotedText) { + return null; + } + const fallback = trimmed.slice(prefix.length).trim(); + return { + emoji, + action: params.actionHint ?? defaultAction, + quotedText: quotedText ?? fallback, + }; + }; + for (const [pattern, { emoji, action }] of TAPBACK_TEXT_MAP) { if (lower.startsWith(pattern)) { // Extract quoted text if present (e.g., 'Loved "hello"' -> "hello") @@ -599,30 +622,14 @@ export function parseTapbackText(params: { } } - if (lower.startsWith("reacted")) { - const emoji = extractFirstEmoji(trimmed) ?? params.emojiHint; - if (!emoji) { - return null; - } - const quotedText = extractQuotedTapbackText(trimmed); - if (params.requireQuoted && !quotedText) { - return null; - } - const fallback = trimmed.slice("reacted".length).trim(); - return { emoji, action: params.actionHint ?? "added", quotedText: quotedText ?? fallback }; + const reacted = parseLeadingReactionAction("reacted", "added"); + if (reacted) { + return reacted; } - if (lower.startsWith("removed")) { - const emoji = extractFirstEmoji(trimmed) ?? params.emojiHint; - if (!emoji) { - return null; - } - const quotedText = extractQuotedTapbackText(trimmed); - if (params.requireQuoted && !quotedText) { - return null; - } - const fallback = trimmed.slice("removed".length).trim(); - return { emoji, action: params.actionHint ?? "removed", quotedText: quotedText ?? fallback }; + const removed = parseLeadingReactionAction("removed", "removed"); + if (removed) { + return removed; } return null; } diff --git a/extensions/bluebubbles/src/monitor.webhook-auth.test.ts b/extensions/bluebubbles/src/monitor.webhook-auth.test.ts index 7a6a29353bd..b72b95dc4cc 100644 --- a/extensions/bluebubbles/src/monitor.webhook-auth.test.ts +++ b/extensions/bluebubbles/src/monitor.webhook-auth.test.ts @@ -302,65 +302,101 @@ describe("BlueBubbles webhook monitor", () => { }; } - describe("webhook parsing + auth handling", () => { - it("rejects non-POST requests", async () => { - const account = createMockAccount(); - const config: OpenClawConfig = {}; - const core = createMockRuntime(); - setBlueBubblesRuntime(core); + async function dispatchWebhook(req: IncomingMessage) { + const res = createMockResponse(); + const handled = await handleBlueBubblesWebhookRequest(req, res); + return { handled, res }; + } - unregister = registerBlueBubblesWebhookTarget({ + function createWebhookRequestForTest(params?: { + method?: string; + url?: string; + body?: unknown; + headers?: Record; + remoteAddress?: string; + }) { + const req = createMockRequest( + params?.method ?? "POST", + params?.url ?? "/bluebubbles-webhook", + params?.body ?? {}, + params?.headers, + ); + if (params?.remoteAddress) { + setRequestRemoteAddress(req, params.remoteAddress); + } + return req; + } + + function createHangingWebhookRequest(url = "/bluebubbles-webhook?password=test-password") { + const req = new EventEmitter() as IncomingMessage & { destroy: ReturnType }; + req.method = "POST"; + req.url = url; + req.headers = {}; + req.destroy = vi.fn(); + setRequestRemoteAddress(req, "127.0.0.1"); + return req; + } + + function registerWebhookTargets( + params: Array<{ + account: ResolvedBlueBubblesAccount; + statusSink?: (event: unknown) => void; + }>, + ) { + const config: OpenClawConfig = {}; + const core = createMockRuntime(); + setBlueBubblesRuntime(core); + + const unregisterFns = params.map(({ account, statusSink }) => + registerBlueBubblesWebhookTarget({ account, config, runtime: { log: vi.fn(), error: vi.fn() }, core, path: "/bluebubbles-webhook", - }); + statusSink, + }), + ); - const req = createMockRequest("GET", "/bluebubbles-webhook", {}); - const res = createMockResponse(); + unregister = () => { + for (const unregisterFn of unregisterFns) { + unregisterFn(); + } + }; + } - const handled = await handleBlueBubblesWebhookRequest(req, res); + async function expectWebhookStatus( + req: IncomingMessage, + expectedStatus: number, + expectedBody?: string, + ) { + const { handled, res } = await dispatchWebhook(req); + expect(handled).toBe(true); + expect(res.statusCode).toBe(expectedStatus); + if (expectedBody !== undefined) { + expect(res.body).toBe(expectedBody); + } + return res; + } - expect(handled).toBe(true); - expect(res.statusCode).toBe(405); + describe("webhook parsing + auth handling", () => { + it("rejects non-POST requests", async () => { + setupWebhookTarget(); + const req = createWebhookRequestForTest({ method: "GET" }); + await expectWebhookStatus(req, 405); }); it("accepts POST requests with valid JSON payload", async () => { setupWebhookTarget(); const payload = createNewMessagePayload({ date: Date.now() }); - - const req = createMockRequest("POST", "/bluebubbles-webhook", payload); - const res = createMockResponse(); - - const handled = await handleBlueBubblesWebhookRequest(req, res); - - expect(handled).toBe(true); - expect(res.statusCode).toBe(200); - expect(res.body).toBe("ok"); + const req = createWebhookRequestForTest({ body: payload }); + await expectWebhookStatus(req, 200, "ok"); }); it("rejects requests with invalid JSON", async () => { - const account = createMockAccount(); - const config: OpenClawConfig = {}; - const core = createMockRuntime(); - setBlueBubblesRuntime(core); - - unregister = registerBlueBubblesWebhookTarget({ - account, - config, - runtime: { log: vi.fn(), error: vi.fn() }, - core, - path: "/bluebubbles-webhook", - }); - - const req = createMockRequest("POST", "/bluebubbles-webhook", "invalid json {{"); - const res = createMockResponse(); - - const handled = await handleBlueBubblesWebhookRequest(req, res); - - expect(handled).toBe(true); - expect(res.statusCode).toBe(400); + setupWebhookTarget(); + const req = createWebhookRequestForTest({ body: "invalid json {{" }); + await expectWebhookStatus(req, 400); }); it("accepts URL-encoded payload wrappers", async () => { @@ -369,42 +405,17 @@ describe("BlueBubbles webhook monitor", () => { const encodedBody = new URLSearchParams({ payload: JSON.stringify(payload), }).toString(); - - const req = createMockRequest("POST", "/bluebubbles-webhook", encodedBody); - const res = createMockResponse(); - - const handled = await handleBlueBubblesWebhookRequest(req, res); - - expect(handled).toBe(true); - expect(res.statusCode).toBe(200); - expect(res.body).toBe("ok"); + const req = createWebhookRequestForTest({ body: encodedBody }); + await expectWebhookStatus(req, 200, "ok"); }); it("returns 408 when request body times out (Slow-Loris protection)", async () => { vi.useFakeTimers(); try { - const account = createMockAccount(); - const config: OpenClawConfig = {}; - const core = createMockRuntime(); - setBlueBubblesRuntime(core); - - unregister = registerBlueBubblesWebhookTarget({ - account, - config, - runtime: { log: vi.fn(), error: vi.fn() }, - core, - path: "/bluebubbles-webhook", - }); + setupWebhookTarget(); // Create a request that never sends data or ends (simulates slow-loris) - const req = new EventEmitter() as IncomingMessage; - req.method = "POST"; - req.url = "/bluebubbles-webhook?password=test-password"; - req.headers = {}; - (req as unknown as { socket: { remoteAddress: string } }).socket = { - remoteAddress: "127.0.0.1", - }; - req.destroy = vi.fn(); + const req = createHangingWebhookRequest(); const res = createMockResponse(); @@ -424,140 +435,62 @@ describe("BlueBubbles webhook monitor", () => { it("rejects unauthorized requests before reading the body", async () => { const account = createMockAccount({ password: "secret-token" }); - const config: OpenClawConfig = {}; - const core = createMockRuntime(); - setBlueBubblesRuntime(core); - - unregister = registerBlueBubblesWebhookTarget({ - account, - config, - runtime: { log: vi.fn(), error: vi.fn() }, - core, - path: "/bluebubbles-webhook", - }); - - const req = new EventEmitter() as IncomingMessage; - req.method = "POST"; - req.url = "/bluebubbles-webhook?password=wrong-token"; - req.headers = {}; + setupWebhookTarget({ account }); + const req = createHangingWebhookRequest("/bluebubbles-webhook?password=wrong-token"); const onSpy = vi.spyOn(req, "on"); - (req as unknown as { socket: { remoteAddress: string } }).socket = { - remoteAddress: "127.0.0.1", - }; - - const res = createMockResponse(); - const handled = await handleBlueBubblesWebhookRequest(req, res); - - expect(handled).toBe(true); - expect(res.statusCode).toBe(401); + await expectWebhookStatus(req, 401); expect(onSpy).not.toHaveBeenCalledWith("data", expect.any(Function)); }); it("authenticates via password query parameter", async () => { const account = createMockAccount({ password: "secret-token" }); - - // Mock non-localhost request - const req = createMockRequest( - "POST", - "/bluebubbles-webhook?password=secret-token", - createNewMessagePayload(), - ); - setRequestRemoteAddress(req, "192.168.1.100"); setupWebhookTarget({ account }); - - const res = createMockResponse(); - const handled = await handleBlueBubblesWebhookRequest(req, res); - - expect(handled).toBe(true); - expect(res.statusCode).toBe(200); + const req = createWebhookRequestForTest({ + url: "/bluebubbles-webhook?password=secret-token", + body: createNewMessagePayload(), + remoteAddress: "192.168.1.100", + }); + await expectWebhookStatus(req, 200); }); it("authenticates via x-password header", async () => { const account = createMockAccount({ password: "secret-token" }); - - const req = createMockRequest( - "POST", - "/bluebubbles-webhook", - createNewMessagePayload(), - { "x-password": "secret-token" }, // pragma: allowlist secret - ); - setRequestRemoteAddress(req, "192.168.1.100"); setupWebhookTarget({ account }); - - const res = createMockResponse(); - const handled = await handleBlueBubblesWebhookRequest(req, res); - - expect(handled).toBe(true); - expect(res.statusCode).toBe(200); + const req = createWebhookRequestForTest({ + body: createNewMessagePayload(), + headers: { "x-password": "secret-token" }, // pragma: allowlist secret + remoteAddress: "192.168.1.100", + }); + await expectWebhookStatus(req, 200); }); it("rejects unauthorized requests with wrong password", async () => { const account = createMockAccount({ password: "secret-token" }); - const req = createMockRequest( - "POST", - "/bluebubbles-webhook?password=wrong-token", - createNewMessagePayload(), - ); - setRequestRemoteAddress(req, "192.168.1.100"); setupWebhookTarget({ account }); - - const res = createMockResponse(); - const handled = await handleBlueBubblesWebhookRequest(req, res); - - expect(handled).toBe(true); - expect(res.statusCode).toBe(401); + const req = createWebhookRequestForTest({ + url: "/bluebubbles-webhook?password=wrong-token", + body: createNewMessagePayload(), + remoteAddress: "192.168.1.100", + }); + await expectWebhookStatus(req, 401); }); it("rejects ambiguous routing when multiple targets match the same password", async () => { const accountA = createMockAccount({ password: "secret-token" }); const accountB = createMockAccount({ password: "secret-token" }); - const config: OpenClawConfig = {}; - const core = createMockRuntime(); - setBlueBubblesRuntime(core); - const sinkA = vi.fn(); const sinkB = vi.fn(); + registerWebhookTargets([ + { account: accountA, statusSink: sinkA }, + { account: accountB, statusSink: sinkB }, + ]); - const req = createMockRequest("POST", "/bluebubbles-webhook?password=secret-token", { - type: "new-message", - data: { - text: "hello", - handle: { address: "+15551234567" }, - isGroup: false, - isFromMe: false, - guid: "msg-1", - }, - }); - (req as unknown as { socket: { remoteAddress: string } }).socket = { + const req = createWebhookRequestForTest({ + url: "/bluebubbles-webhook?password=secret-token", + body: createNewMessagePayload(), remoteAddress: "192.168.1.100", - }; - - const unregisterA = registerBlueBubblesWebhookTarget({ - account: accountA, - config, - runtime: { log: vi.fn(), error: vi.fn() }, - core, - path: "/bluebubbles-webhook", - statusSink: sinkA, }); - const unregisterB = registerBlueBubblesWebhookTarget({ - account: accountB, - config, - runtime: { log: vi.fn(), error: vi.fn() }, - core, - path: "/bluebubbles-webhook", - statusSink: sinkB, - }); - unregister = () => { - unregisterA(); - unregisterB(); - }; - - const res = createMockResponse(); - const handled = await handleBlueBubblesWebhookRequest(req, res); - - expect(handled).toBe(true); - expect(res.statusCode).toBe(401); + await expectWebhookStatus(req, 401); expect(sinkA).not.toHaveBeenCalled(); expect(sinkB).not.toHaveBeenCalled(); }); @@ -565,107 +498,38 @@ describe("BlueBubbles webhook monitor", () => { it("ignores targets without passwords when a password-authenticated target matches", async () => { const accountStrict = createMockAccount({ password: "secret-token" }); const accountWithoutPassword = createMockAccount({ password: undefined }); - const config: OpenClawConfig = {}; - const core = createMockRuntime(); - setBlueBubblesRuntime(core); - const sinkStrict = vi.fn(); const sinkWithoutPassword = vi.fn(); + registerWebhookTargets([ + { account: accountStrict, statusSink: sinkStrict }, + { account: accountWithoutPassword, statusSink: sinkWithoutPassword }, + ]); - const req = createMockRequest("POST", "/bluebubbles-webhook?password=secret-token", { - type: "new-message", - data: { - text: "hello", - handle: { address: "+15551234567" }, - isGroup: false, - isFromMe: false, - guid: "msg-1", - }, - }); - (req as unknown as { socket: { remoteAddress: string } }).socket = { + const req = createWebhookRequestForTest({ + url: "/bluebubbles-webhook?password=secret-token", + body: createNewMessagePayload(), remoteAddress: "192.168.1.100", - }; - - const unregisterStrict = registerBlueBubblesWebhookTarget({ - account: accountStrict, - config, - runtime: { log: vi.fn(), error: vi.fn() }, - core, - path: "/bluebubbles-webhook", - statusSink: sinkStrict, }); - const unregisterNoPassword = registerBlueBubblesWebhookTarget({ - account: accountWithoutPassword, - config, - runtime: { log: vi.fn(), error: vi.fn() }, - core, - path: "/bluebubbles-webhook", - statusSink: sinkWithoutPassword, - }); - unregister = () => { - unregisterStrict(); - unregisterNoPassword(); - }; - - const res = createMockResponse(); - const handled = await handleBlueBubblesWebhookRequest(req, res); - - expect(handled).toBe(true); - expect(res.statusCode).toBe(200); + await expectWebhookStatus(req, 200); expect(sinkStrict).toHaveBeenCalledTimes(1); expect(sinkWithoutPassword).not.toHaveBeenCalled(); }); it("requires authentication for loopback requests when password is configured", async () => { const account = createMockAccount({ password: "secret-token" }); - const config: OpenClawConfig = {}; - const core = createMockRuntime(); - setBlueBubblesRuntime(core); + setupWebhookTarget({ account }); for (const remoteAddress of ["127.0.0.1", "::1", "::ffff:127.0.0.1"]) { - const req = createMockRequest("POST", "/bluebubbles-webhook", { - type: "new-message", - data: { - text: "hello", - handle: { address: "+15551234567" }, - isGroup: false, - isFromMe: false, - guid: "msg-1", - }, - }); - (req as unknown as { socket: { remoteAddress: string } }).socket = { + const req = createWebhookRequestForTest({ + body: createNewMessagePayload(), remoteAddress, - }; - - const loopbackUnregister = registerBlueBubblesWebhookTarget({ - account, - config, - runtime: { log: vi.fn(), error: vi.fn() }, - core, - path: "/bluebubbles-webhook", }); - - const res = createMockResponse(); - const handled = await handleBlueBubblesWebhookRequest(req, res); - expect(handled).toBe(true); - expect(res.statusCode).toBe(401); - - loopbackUnregister(); + await expectWebhookStatus(req, 401); } }); it("rejects targets without passwords for loopback and proxied-looking requests", async () => { const account = createMockAccount({ password: undefined }); - const config: OpenClawConfig = {}; - const core = createMockRuntime(); - setBlueBubblesRuntime(core); - - unregister = registerBlueBubblesWebhookTarget({ - account, - config, - runtime: { log: vi.fn(), error: vi.fn() }, - core, - path: "/bluebubbles-webhook", - }); + setupWebhookTarget({ account }); const headerVariants: Record[] = [ { host: "localhost" }, @@ -673,28 +537,12 @@ describe("BlueBubbles webhook monitor", () => { { host: "localhost", forwarded: "for=203.0.113.10;proto=https;host=example.com" }, ]; for (const headers of headerVariants) { - const req = createMockRequest( - "POST", - "/bluebubbles-webhook", - { - type: "new-message", - data: { - text: "hello", - handle: { address: "+15551234567" }, - isGroup: false, - isFromMe: false, - guid: "msg-1", - }, - }, + const req = createWebhookRequestForTest({ + body: createNewMessagePayload(), headers, - ); - (req as unknown as { socket: { remoteAddress: string } }).socket = { remoteAddress: "127.0.0.1", - }; - const res = createMockResponse(); - const handled = await handleBlueBubblesWebhookRequest(req, res); - expect(handled).toBe(true); - expect(res.statusCode).toBe(401); + }); + await expectWebhookStatus(req, 401); } }); diff --git a/extensions/bluebubbles/src/reactions.test.ts b/extensions/bluebubbles/src/reactions.test.ts index 419ccc81e45..0b55337b35c 100644 --- a/extensions/bluebubbles/src/reactions.test.ts +++ b/extensions/bluebubbles/src/reactions.test.ts @@ -19,7 +19,7 @@ describe("reactions", () => { }); describe("sendBlueBubblesReaction", () => { - async function expectRemovedReaction(emoji: string) { + async function expectRemovedReaction(emoji: string, expectedReaction = "-love") { mockFetch.mockResolvedValueOnce({ ok: true, text: () => Promise.resolve(""), @@ -37,7 +37,7 @@ describe("reactions", () => { }); const body = JSON.parse(mockFetch.mock.calls[0][1].body); - expect(body.reaction).toBe("-love"); + expect(body.reaction).toBe(expectedReaction); } it("throws when chatGuid is empty", async () => { @@ -327,45 +327,11 @@ describe("reactions", () => { describe("reaction removal aliases", () => { it("handles emoji-based removal", async () => { - mockFetch.mockResolvedValueOnce({ - ok: true, - text: () => Promise.resolve(""), - }); - - await sendBlueBubblesReaction({ - chatGuid: "chat-123", - messageGuid: "msg-123", - emoji: "👍", - remove: true, - opts: { - serverUrl: "http://localhost:1234", - password: "test", - }, - }); - - const body = JSON.parse(mockFetch.mock.calls[0][1].body); - expect(body.reaction).toBe("-like"); + await expectRemovedReaction("👍", "-like"); }); it("handles text alias removal", async () => { - mockFetch.mockResolvedValueOnce({ - ok: true, - text: () => Promise.resolve(""), - }); - - await sendBlueBubblesReaction({ - chatGuid: "chat-123", - messageGuid: "msg-123", - emoji: "haha", - remove: true, - opts: { - serverUrl: "http://localhost:1234", - password: "test", - }, - }); - - const body = JSON.parse(mockFetch.mock.calls[0][1].body); - expect(body.reaction).toBe("-laugh"); + await expectRemovedReaction("haha", "-laugh"); }); }); }); diff --git a/extensions/feishu/src/reply-dispatcher.test.ts b/extensions/feishu/src/reply-dispatcher.test.ts index 744532320de..e6f7fd4d974 100644 --- a/extensions/feishu/src/reply-dispatcher.test.ts +++ b/extensions/feishu/src/reply-dispatcher.test.ts @@ -63,6 +63,8 @@ vi.mock("./streaming-card.js", () => ({ import { createFeishuReplyDispatcher } from "./reply-dispatcher.js"; describe("createFeishuReplyDispatcher streaming behavior", () => { + type ReplyDispatcherArgs = Parameters[0]; + beforeEach(() => { vi.clearAllMocks(); streamingInstances.length = 0; @@ -128,6 +130,25 @@ describe("createFeishuReplyDispatcher streaming behavior", () => { return createReplyDispatcherWithTypingMock.mock.calls[0]?.[0]; } + function createRuntimeLogger() { + return { log: vi.fn(), error: vi.fn() } as never; + } + + function createDispatcherHarness(overrides: Partial = {}) { + const result = createFeishuReplyDispatcher({ + cfg: {} as never, + agentId: "agent", + runtime: {} as never, + chatId: "oc_chat", + ...overrides, + }); + + return { + result, + options: createReplyDispatcherWithTypingMock.mock.calls.at(-1)?.[0], + }; + } + it("skips typing indicator when account typingIndicator is disabled", async () => { resolveFeishuAccountMock.mockReturnValue({ accountId: "main", @@ -209,14 +230,7 @@ describe("createFeishuReplyDispatcher streaming behavior", () => { }); it("keeps auto mode plain text on non-streaming send path", async () => { - createFeishuReplyDispatcher({ - cfg: {} as never, - agentId: "agent", - runtime: {} as never, - chatId: "oc_chat", - }); - - const options = createReplyDispatcherWithTypingMock.mock.calls[0]?.[0]; + const { options } = createDispatcherHarness(); await options.deliver({ text: "plain text" }, { kind: "final" }); expect(streamingInstances).toHaveLength(0); @@ -225,14 +239,7 @@ describe("createFeishuReplyDispatcher streaming behavior", () => { }); it("suppresses internal block payload delivery", async () => { - createFeishuReplyDispatcher({ - cfg: {} as never, - agentId: "agent", - runtime: {} as never, - chatId: "oc_chat", - }); - - const options = createReplyDispatcherWithTypingMock.mock.calls[0]?.[0]; + const { options } = createDispatcherHarness(); await options.deliver({ text: "internal reasoning chunk" }, { kind: "block" }); expect(streamingInstances).toHaveLength(0); @@ -253,15 +260,10 @@ describe("createFeishuReplyDispatcher streaming behavior", () => { }); it("uses streaming session for auto mode markdown payloads", async () => { - createFeishuReplyDispatcher({ - cfg: {} as never, - agentId: "agent", - runtime: { log: vi.fn(), error: vi.fn() } as never, - chatId: "oc_chat", + const { options } = createDispatcherHarness({ + runtime: createRuntimeLogger(), rootId: "om_root_topic", }); - - const options = createReplyDispatcherWithTypingMock.mock.calls[0]?.[0]; await options.deliver({ text: "```ts\nconst x = 1\n```" }, { kind: "final" }); expect(streamingInstances).toHaveLength(1); @@ -277,14 +279,9 @@ describe("createFeishuReplyDispatcher streaming behavior", () => { }); it("closes streaming with block text when final reply is missing", async () => { - createFeishuReplyDispatcher({ - cfg: {} as never, - agentId: "agent", - runtime: { log: vi.fn(), error: vi.fn() } as never, - chatId: "oc_chat", + const { options } = createDispatcherHarness({ + runtime: createRuntimeLogger(), }); - - const options = createReplyDispatcherWithTypingMock.mock.calls[0]?.[0]; await options.deliver({ text: "```md\npartial answer\n```" }, { kind: "block" }); await options.onIdle?.(); @@ -295,14 +292,9 @@ describe("createFeishuReplyDispatcher streaming behavior", () => { }); it("delivers distinct final payloads after streaming close", async () => { - createFeishuReplyDispatcher({ - cfg: {} as never, - agentId: "agent", - runtime: { log: vi.fn(), error: vi.fn() } as never, - chatId: "oc_chat", + const { options } = createDispatcherHarness({ + runtime: createRuntimeLogger(), }); - - const options = createReplyDispatcherWithTypingMock.mock.calls[0]?.[0]; await options.deliver({ text: "```md\n完整回复第一段\n```" }, { kind: "final" }); await options.deliver({ text: "```md\n完整回复第一段 + 第二段\n```" }, { kind: "final" }); @@ -316,14 +308,9 @@ describe("createFeishuReplyDispatcher streaming behavior", () => { }); it("skips exact duplicate final text after streaming close", async () => { - createFeishuReplyDispatcher({ - cfg: {} as never, - agentId: "agent", - runtime: { log: vi.fn(), error: vi.fn() } as never, - chatId: "oc_chat", + const { options } = createDispatcherHarness({ + runtime: createRuntimeLogger(), }); - - const options = createReplyDispatcherWithTypingMock.mock.calls[0]?.[0]; await options.deliver({ text: "```md\n同一条回复\n```" }, { kind: "final" }); await options.deliver({ text: "```md\n同一条回复\n```" }, { kind: "final" }); @@ -383,14 +370,9 @@ describe("createFeishuReplyDispatcher streaming behavior", () => { }, }); - const result = createFeishuReplyDispatcher({ - cfg: {} as never, - agentId: "agent", - runtime: { log: vi.fn(), error: vi.fn() } as never, - chatId: "oc_chat", + const { result, options } = createDispatcherHarness({ + runtime: createRuntimeLogger(), }); - - const options = createReplyDispatcherWithTypingMock.mock.calls[0]?.[0]; await options.onReplyStart?.(); await result.replyOptions.onPartialReply?.({ text: "hello" }); await options.deliver({ text: "lo world" }, { kind: "block" }); @@ -402,14 +384,7 @@ describe("createFeishuReplyDispatcher streaming behavior", () => { }); it("sends media-only payloads as attachments", async () => { - createFeishuReplyDispatcher({ - cfg: {} as never, - agentId: "agent", - runtime: {} as never, - chatId: "oc_chat", - }); - - const options = createReplyDispatcherWithTypingMock.mock.calls[0]?.[0]; + const { options } = createDispatcherHarness(); await options.deliver({ mediaUrl: "https://example.com/a.png" }, { kind: "final" }); expect(sendMediaFeishuMock).toHaveBeenCalledTimes(1); @@ -424,14 +399,7 @@ describe("createFeishuReplyDispatcher streaming behavior", () => { }); it("falls back to legacy mediaUrl when mediaUrls is an empty array", async () => { - createFeishuReplyDispatcher({ - cfg: {} as never, - agentId: "agent", - runtime: {} as never, - chatId: "oc_chat", - }); - - const options = createReplyDispatcherWithTypingMock.mock.calls[0]?.[0]; + const { options } = createDispatcherHarness(); await options.deliver( { text: "caption", mediaUrl: "https://example.com/a.png", mediaUrls: [] }, { kind: "final" }, @@ -447,14 +415,9 @@ describe("createFeishuReplyDispatcher streaming behavior", () => { }); it("sends attachments after streaming final markdown replies", async () => { - createFeishuReplyDispatcher({ - cfg: {} as never, - agentId: "agent", - runtime: { log: vi.fn(), error: vi.fn() } as never, - chatId: "oc_chat", + const { options } = createDispatcherHarness({ + runtime: createRuntimeLogger(), }); - - const options = createReplyDispatcherWithTypingMock.mock.calls[0]?.[0]; await options.deliver( { text: "```ts\nconst x = 1\n```", mediaUrls: ["https://example.com/a.png"] }, { kind: "final" }, @@ -472,16 +435,10 @@ describe("createFeishuReplyDispatcher streaming behavior", () => { }); it("passes replyInThread to sendMessageFeishu for plain text", async () => { - createFeishuReplyDispatcher({ - cfg: {} as never, - agentId: "agent", - runtime: {} as never, - chatId: "oc_chat", + const { options } = createDispatcherHarness({ replyToMessageId: "om_msg", replyInThread: true, }); - - const options = createReplyDispatcherWithTypingMock.mock.calls[0]?.[0]; await options.deliver({ text: "plain text" }, { kind: "final" }); expect(sendMessageFeishuMock).toHaveBeenCalledWith( @@ -504,16 +461,10 @@ describe("createFeishuReplyDispatcher streaming behavior", () => { }, }); - createFeishuReplyDispatcher({ - cfg: {} as never, - agentId: "agent", - runtime: {} as never, - chatId: "oc_chat", + const { options } = createDispatcherHarness({ replyToMessageId: "om_msg", replyInThread: true, }); - - const options = createReplyDispatcherWithTypingMock.mock.calls[0]?.[0]; await options.deliver({ text: "card text" }, { kind: "final" }); expect(sendMarkdownCardFeishuMock).toHaveBeenCalledWith( @@ -525,16 +476,11 @@ describe("createFeishuReplyDispatcher streaming behavior", () => { }); it("passes replyToMessageId and replyInThread to streaming.start()", async () => { - createFeishuReplyDispatcher({ - cfg: {} as never, - agentId: "agent", - runtime: { log: vi.fn(), error: vi.fn() } as never, - chatId: "oc_chat", + const { options } = createDispatcherHarness({ + runtime: createRuntimeLogger(), replyToMessageId: "om_msg", replyInThread: true, }); - - const options = createReplyDispatcherWithTypingMock.mock.calls[0]?.[0]; await options.deliver({ text: "```ts\nconst x = 1\n```" }, { kind: "final" }); expect(streamingInstances).toHaveLength(1); @@ -545,18 +491,13 @@ describe("createFeishuReplyDispatcher streaming behavior", () => { }); it("disables streaming for thread replies and keeps reply metadata", async () => { - createFeishuReplyDispatcher({ - cfg: {} as never, - agentId: "agent", - runtime: { log: vi.fn(), error: vi.fn() } as never, - chatId: "oc_chat", + const { options } = createDispatcherHarness({ + runtime: createRuntimeLogger(), replyToMessageId: "om_msg", replyInThread: false, threadReply: true, rootId: "om_root_topic", }); - - const options = createReplyDispatcherWithTypingMock.mock.calls[0]?.[0]; await options.deliver({ text: "```ts\nconst x = 1\n```" }, { kind: "final" }); expect(streamingInstances).toHaveLength(0); @@ -569,16 +510,10 @@ describe("createFeishuReplyDispatcher streaming behavior", () => { }); it("passes replyInThread to media attachments", async () => { - createFeishuReplyDispatcher({ - cfg: {} as never, - agentId: "agent", - runtime: {} as never, - chatId: "oc_chat", + const { options } = createDispatcherHarness({ replyToMessageId: "om_msg", replyInThread: true, }); - - const options = createReplyDispatcherWithTypingMock.mock.calls[0]?.[0]; await options.deliver({ mediaUrl: "https://example.com/a.png" }, { kind: "final" }); expect(sendMediaFeishuMock).toHaveBeenCalledWith( diff --git a/extensions/feishu/src/reply-dispatcher.ts b/extensions/feishu/src/reply-dispatcher.ts index 3bd1353825d..6f66ffffa58 100644 --- a/extensions/feishu/src/reply-dispatcher.ts +++ b/extensions/feishu/src/reply-dispatcher.ts @@ -224,6 +224,41 @@ export function createFeishuReplyDispatcher(params: CreateFeishuReplyDispatcherP lastPartial = ""; }; + const sendChunkedTextReply = async (params: { + text: string; + useCard: boolean; + infoKind?: string; + }) => { + let first = true; + const chunkSource = params.useCard + ? params.text + : core.channel.text.convertMarkdownTables(params.text, tableMode); + for (const chunk of core.channel.text.chunkTextWithMode( + chunkSource, + textChunkLimit, + chunkMode, + )) { + const message = { + cfg, + to: chatId, + text: chunk, + replyToMessageId: sendReplyToMessageId, + replyInThread: effectiveReplyInThread, + mentions: first ? mentionTargets : undefined, + accountId, + }; + if (params.useCard) { + await sendMarkdownCardFeishu(message); + } else { + await sendMessageFeishu(message); + } + first = false; + } + if (params.infoKind === "final") { + deliveredFinalTexts.add(params.text); + } + }; + const { dispatcher, replyOptions, markDispatchIdle } = core.channel.reply.createReplyDispatcherWithTyping({ responsePrefix: prefixContext.responsePrefix, @@ -303,48 +338,10 @@ export function createFeishuReplyDispatcher(params: CreateFeishuReplyDispatcherP return; } - let first = true; if (useCard) { - for (const chunk of core.channel.text.chunkTextWithMode( - text, - textChunkLimit, - chunkMode, - )) { - await sendMarkdownCardFeishu({ - cfg, - to: chatId, - text: chunk, - replyToMessageId: sendReplyToMessageId, - replyInThread: effectiveReplyInThread, - mentions: first ? mentionTargets : undefined, - accountId, - }); - first = false; - } - if (info?.kind === "final") { - deliveredFinalTexts.add(text); - } + await sendChunkedTextReply({ text, useCard: true, infoKind: info?.kind }); } else { - const converted = core.channel.text.convertMarkdownTables(text, tableMode); - for (const chunk of core.channel.text.chunkTextWithMode( - converted, - textChunkLimit, - chunkMode, - )) { - await sendMessageFeishu({ - cfg, - to: chatId, - text: chunk, - replyToMessageId: sendReplyToMessageId, - replyInThread: effectiveReplyInThread, - mentions: first ? mentionTargets : undefined, - accountId, - }); - first = false; - } - if (info?.kind === "final") { - deliveredFinalTexts.add(text); - } + await sendChunkedTextReply({ text, useCard: false, infoKind: info?.kind }); } } diff --git a/extensions/feishu/src/send.reply-fallback.test.ts b/extensions/feishu/src/send.reply-fallback.test.ts index 75dda353bbe..610ded167fd 100644 --- a/extensions/feishu/src/send.reply-fallback.test.ts +++ b/extensions/feishu/src/send.reply-fallback.test.ts @@ -25,6 +25,16 @@ describe("Feishu reply fallback for withdrawn/deleted targets", () => { const replyMock = vi.fn(); const createMock = vi.fn(); + async function expectFallbackResult( + send: () => Promise<{ messageId?: string }>, + expectedMessageId: string, + ) { + const result = await send(); + expect(replyMock).toHaveBeenCalledTimes(1); + expect(createMock).toHaveBeenCalledTimes(1); + expect(result.messageId).toBe(expectedMessageId); + } + beforeEach(() => { vi.clearAllMocks(); resolveFeishuSendTargetMock.mockReturnValue({ @@ -51,16 +61,16 @@ describe("Feishu reply fallback for withdrawn/deleted targets", () => { data: { message_id: "om_new" }, }); - const result = await sendMessageFeishu({ - cfg: {} as never, - to: "user:ou_target", - text: "hello", - replyToMessageId: "om_parent", - }); - - expect(replyMock).toHaveBeenCalledTimes(1); - expect(createMock).toHaveBeenCalledTimes(1); - expect(result.messageId).toBe("om_new"); + await expectFallbackResult( + () => + sendMessageFeishu({ + cfg: {} as never, + to: "user:ou_target", + text: "hello", + replyToMessageId: "om_parent", + }), + "om_new", + ); }); it("falls back to create for withdrawn card replies", async () => { @@ -73,16 +83,16 @@ describe("Feishu reply fallback for withdrawn/deleted targets", () => { data: { message_id: "om_card_new" }, }); - const result = await sendCardFeishu({ - cfg: {} as never, - to: "user:ou_target", - card: { schema: "2.0" }, - replyToMessageId: "om_parent", - }); - - expect(replyMock).toHaveBeenCalledTimes(1); - expect(createMock).toHaveBeenCalledTimes(1); - expect(result.messageId).toBe("om_card_new"); + await expectFallbackResult( + () => + sendCardFeishu({ + cfg: {} as never, + to: "user:ou_target", + card: { schema: "2.0" }, + replyToMessageId: "om_parent", + }), + "om_card_new", + ); }); it("still throws for non-withdrawn reply failures", async () => { @@ -111,16 +121,16 @@ describe("Feishu reply fallback for withdrawn/deleted targets", () => { data: { message_id: "om_thrown_fallback" }, }); - const result = await sendMessageFeishu({ - cfg: {} as never, - to: "user:ou_target", - text: "hello", - replyToMessageId: "om_parent", - }); - - expect(replyMock).toHaveBeenCalledTimes(1); - expect(createMock).toHaveBeenCalledTimes(1); - expect(result.messageId).toBe("om_thrown_fallback"); + await expectFallbackResult( + () => + sendMessageFeishu({ + cfg: {} as never, + to: "user:ou_target", + text: "hello", + replyToMessageId: "om_parent", + }), + "om_thrown_fallback", + ); }); it("falls back to create when card reply throws a not-found AxiosError", async () => { @@ -133,16 +143,16 @@ describe("Feishu reply fallback for withdrawn/deleted targets", () => { data: { message_id: "om_axios_fallback" }, }); - const result = await sendCardFeishu({ - cfg: {} as never, - to: "user:ou_target", - card: { schema: "2.0" }, - replyToMessageId: "om_parent", - }); - - expect(replyMock).toHaveBeenCalledTimes(1); - expect(createMock).toHaveBeenCalledTimes(1); - expect(result.messageId).toBe("om_axios_fallback"); + await expectFallbackResult( + () => + sendCardFeishu({ + cfg: {} as never, + to: "user:ou_target", + card: { schema: "2.0" }, + replyToMessageId: "om_parent", + }), + "om_axios_fallback", + ); }); it("re-throws non-withdrawn thrown errors for text messages", async () => { diff --git a/extensions/feishu/src/send.ts b/extensions/feishu/src/send.ts index 5bfa836e0a6..5692edd32ff 100644 --- a/extensions/feishu/src/send.ts +++ b/extensions/feishu/src/send.ts @@ -55,6 +55,30 @@ type FeishuCreateMessageClient = { }; }; +type FeishuMessageSender = { + id?: string; + id_type?: string; + sender_type?: string; +}; + +type FeishuMessageGetItem = { + message_id?: string; + chat_id?: string; + chat_type?: FeishuChatType; + msg_type?: string; + body?: { content?: string }; + sender?: FeishuMessageSender; + create_time?: string; +}; + +type FeishuGetMessageResponse = { + code?: number; + msg?: string; + data?: FeishuMessageGetItem & { + items?: FeishuMessageGetItem[]; + }; +}; + /** Send a direct message as a fallback when a reply target is unavailable. */ async function sendFallbackDirect( client: FeishuCreateMessageClient, @@ -214,36 +238,7 @@ export async function getMessageFeishu(params: { try { const response = (await client.im.message.get({ path: { message_id: messageId }, - })) as { - code?: number; - msg?: string; - data?: { - items?: Array<{ - message_id?: string; - chat_id?: string; - chat_type?: FeishuChatType; - msg_type?: string; - body?: { content?: string }; - sender?: { - id?: string; - id_type?: string; - sender_type?: string; - }; - create_time?: string; - }>; - message_id?: string; - chat_id?: string; - chat_type?: FeishuChatType; - msg_type?: string; - body?: { content?: string }; - sender?: { - id?: string; - id_type?: string; - sender_type?: string; - }; - create_time?: string; - }; - }; + })) as FeishuGetMessageResponse; if (response.code !== 0) { return null; diff --git a/extensions/googlechat/src/api.test.ts b/extensions/googlechat/src/api.test.ts index fc011268ec2..81312d39820 100644 --- a/extensions/googlechat/src/api.test.ts +++ b/extensions/googlechat/src/api.test.ts @@ -13,6 +13,21 @@ const account = { config: {}, } as ResolvedGoogleChatAccount; +function stubSuccessfulSend(name: string) { + const fetchMock = vi + .fn() + .mockResolvedValue(new Response(JSON.stringify({ name }), { status: 200 })); + vi.stubGlobal("fetch", fetchMock); + return fetchMock; +} + +async function expectDownloadToRejectForResponse(response: Response) { + vi.stubGlobal("fetch", vi.fn().mockResolvedValue(response)); + await expect( + downloadGoogleChatMedia({ account, resourceName: "media/123", maxBytes: 10 }), + ).rejects.toThrow(/max bytes/i); +} + describe("downloadGoogleChatMedia", () => { afterEach(() => { vi.unstubAllGlobals(); @@ -29,11 +44,7 @@ describe("downloadGoogleChatMedia", () => { status: 200, headers: { "content-length": "50", "content-type": "application/octet-stream" }, }); - vi.stubGlobal("fetch", vi.fn().mockResolvedValue(response)); - - await expect( - downloadGoogleChatMedia({ account, resourceName: "media/123", maxBytes: 10 }), - ).rejects.toThrow(/max bytes/i); + await expectDownloadToRejectForResponse(response); }); it("rejects when streamed payload exceeds max bytes", async () => { @@ -52,11 +63,7 @@ describe("downloadGoogleChatMedia", () => { status: 200, headers: { "content-type": "application/octet-stream" }, }); - vi.stubGlobal("fetch", vi.fn().mockResolvedValue(response)); - - await expect( - downloadGoogleChatMedia({ account, resourceName: "media/123", maxBytes: 10 }), - ).rejects.toThrow(/max bytes/i); + await expectDownloadToRejectForResponse(response); }); }); @@ -66,12 +73,7 @@ describe("sendGoogleChatMessage", () => { }); it("adds messageReplyOption when sending to an existing thread", async () => { - const fetchMock = vi - .fn() - .mockResolvedValue( - new Response(JSON.stringify({ name: "spaces/AAA/messages/123" }), { status: 200 }), - ); - vi.stubGlobal("fetch", fetchMock); + const fetchMock = stubSuccessfulSend("spaces/AAA/messages/123"); await sendGoogleChatMessage({ account, @@ -89,12 +91,7 @@ describe("sendGoogleChatMessage", () => { }); it("does not set messageReplyOption for non-thread sends", async () => { - const fetchMock = vi - .fn() - .mockResolvedValue( - new Response(JSON.stringify({ name: "spaces/AAA/messages/124" }), { status: 200 }), - ); - vi.stubGlobal("fetch", fetchMock); + const fetchMock = stubSuccessfulSend("spaces/AAA/messages/124"); await sendGoogleChatMessage({ account, diff --git a/extensions/googlechat/src/api.ts b/extensions/googlechat/src/api.ts index 7c4f26b8db9..d9c7b666ff0 100644 --- a/extensions/googlechat/src/api.ts +++ b/extensions/googlechat/src/api.ts @@ -14,70 +14,24 @@ const headersToObject = (headers?: HeadersInit): Record => ? Object.fromEntries(headers) : headers || {}; -async function fetchJson( - account: ResolvedGoogleChatAccount, - url: string, - init: RequestInit, -): Promise { - const token = await getGoogleChatAccessToken(account); - const { response: res, release } = await fetchWithSsrFGuard({ +async function withGoogleChatResponse(params: { + account: ResolvedGoogleChatAccount; + url: string; + init?: RequestInit; + auditContext: string; + errorPrefix?: string; + handleResponse: (response: Response) => Promise; +}): Promise { + const { + account, url, - init: { - ...init, - headers: { - ...headersToObject(init.headers), - Authorization: `Bearer ${token}`, - "Content-Type": "application/json", - }, - }, - auditContext: "googlechat.api.json", - }); - try { - if (!res.ok) { - const text = await res.text().catch(() => ""); - throw new Error(`Google Chat API ${res.status}: ${text || res.statusText}`); - } - return (await res.json()) as T; - } finally { - await release(); - } -} - -async function fetchOk( - account: ResolvedGoogleChatAccount, - url: string, - init: RequestInit, -): Promise { + init, + auditContext, + errorPrefix = "Google Chat API", + handleResponse, + } = params; const token = await getGoogleChatAccessToken(account); - const { response: res, release } = await fetchWithSsrFGuard({ - url, - init: { - ...init, - headers: { - ...headersToObject(init.headers), - Authorization: `Bearer ${token}`, - }, - }, - auditContext: "googlechat.api.ok", - }); - try { - if (!res.ok) { - const text = await res.text().catch(() => ""); - throw new Error(`Google Chat API ${res.status}: ${text || res.statusText}`); - } - } finally { - await release(); - } -} - -async function fetchBuffer( - account: ResolvedGoogleChatAccount, - url: string, - init?: RequestInit, - options?: { maxBytes?: number }, -): Promise<{ buffer: Buffer; contentType?: string }> { - const token = await getGoogleChatAccessToken(account); - const { response: res, release } = await fetchWithSsrFGuard({ + const { response, release } = await fetchWithSsrFGuard({ url, init: { ...init, @@ -86,52 +40,103 @@ async function fetchBuffer( Authorization: `Bearer ${token}`, }, }, - auditContext: "googlechat.api.buffer", + auditContext, }); try { - if (!res.ok) { - const text = await res.text().catch(() => ""); - throw new Error(`Google Chat API ${res.status}: ${text || res.statusText}`); + if (!response.ok) { + const text = await response.text().catch(() => ""); + throw new Error(`${errorPrefix} ${response.status}: ${text || response.statusText}`); } - const maxBytes = options?.maxBytes; - const lengthHeader = res.headers.get("content-length"); - if (maxBytes && lengthHeader) { - const length = Number(lengthHeader); - if (Number.isFinite(length) && length > maxBytes) { - throw new Error(`Google Chat media exceeds max bytes (${maxBytes})`); - } - } - if (!maxBytes || !res.body) { - const buffer = Buffer.from(await res.arrayBuffer()); - const contentType = res.headers.get("content-type") ?? undefined; - return { buffer, contentType }; - } - const reader = res.body.getReader(); - const chunks: Buffer[] = []; - let total = 0; - while (true) { - const { done, value } = await reader.read(); - if (done) { - break; - } - if (!value) { - continue; - } - total += value.length; - if (total > maxBytes) { - await reader.cancel(); - throw new Error(`Google Chat media exceeds max bytes (${maxBytes})`); - } - chunks.push(Buffer.from(value)); - } - const buffer = Buffer.concat(chunks, total); - const contentType = res.headers.get("content-type") ?? undefined; - return { buffer, contentType }; + return await handleResponse(response); } finally { await release(); } } +async function fetchJson( + account: ResolvedGoogleChatAccount, + url: string, + init: RequestInit, +): Promise { + return await withGoogleChatResponse({ + account, + url, + init: { + ...init, + headers: { + ...headersToObject(init.headers), + "Content-Type": "application/json", + }, + }, + auditContext: "googlechat.api.json", + handleResponse: async (response) => (await response.json()) as T, + }); +} + +async function fetchOk( + account: ResolvedGoogleChatAccount, + url: string, + init: RequestInit, +): Promise { + await withGoogleChatResponse({ + account, + url, + init, + auditContext: "googlechat.api.ok", + handleResponse: async () => undefined, + }); +} + +async function fetchBuffer( + account: ResolvedGoogleChatAccount, + url: string, + init?: RequestInit, + options?: { maxBytes?: number }, +): Promise<{ buffer: Buffer; contentType?: string }> { + return await withGoogleChatResponse({ + account, + url, + init, + auditContext: "googlechat.api.buffer", + handleResponse: async (res) => { + const maxBytes = options?.maxBytes; + const lengthHeader = res.headers.get("content-length"); + if (maxBytes && lengthHeader) { + const length = Number(lengthHeader); + if (Number.isFinite(length) && length > maxBytes) { + throw new Error(`Google Chat media exceeds max bytes (${maxBytes})`); + } + } + if (!maxBytes || !res.body) { + const buffer = Buffer.from(await res.arrayBuffer()); + const contentType = res.headers.get("content-type") ?? undefined; + return { buffer, contentType }; + } + const reader = res.body.getReader(); + const chunks: Buffer[] = []; + let total = 0; + while (true) { + const { done, value } = await reader.read(); + if (done) { + break; + } + if (!value) { + continue; + } + total += value.length; + if (total > maxBytes) { + await reader.cancel(); + throw new Error(`Google Chat media exceeds max bytes (${maxBytes})`); + } + chunks.push(Buffer.from(value)); + } + const buffer = Buffer.concat(chunks, total); + const contentType = res.headers.get("content-type") ?? undefined; + return { buffer, contentType }; + }, + }); +} + export async function sendGoogleChatMessage(params: { account: ResolvedGoogleChatAccount; space: string; @@ -208,34 +213,29 @@ export async function uploadGoogleChatAttachment(params: { Buffer.from(footer, "utf8"), ]); - const token = await getGoogleChatAccessToken(account); const url = `${CHAT_UPLOAD_BASE}/${space}/attachments:upload?uploadType=multipart`; - const { response: res, release } = await fetchWithSsrFGuard({ + const payload = await withGoogleChatResponse<{ + attachmentDataRef?: { attachmentUploadToken?: string }; + }>({ + account, url, init: { method: "POST", headers: { - Authorization: `Bearer ${token}`, "Content-Type": `multipart/related; boundary=${boundary}`, }, body, }, auditContext: "googlechat.upload", + errorPrefix: "Google Chat upload", + handleResponse: async (response) => + (await response.json()) as { + attachmentDataRef?: { attachmentUploadToken?: string }; + }, }); - try { - if (!res.ok) { - const text = await res.text().catch(() => ""); - throw new Error(`Google Chat upload ${res.status}: ${text || res.statusText}`); - } - const payload = (await res.json()) as { - attachmentDataRef?: { attachmentUploadToken?: string }; - }; - return { - attachmentUploadToken: payload.attachmentDataRef?.attachmentUploadToken, - }; - } finally { - await release(); - } + return { + attachmentUploadToken: payload.attachmentDataRef?.attachmentUploadToken, + }; } export async function downloadGoogleChatMedia(params: { diff --git a/extensions/googlechat/src/monitor.webhook-routing.test.ts b/extensions/googlechat/src/monitor.webhook-routing.test.ts index 812883f1b4c..9896efce645 100644 --- a/extensions/googlechat/src/monitor.webhook-routing.test.ts +++ b/extensions/googlechat/src/monitor.webhook-routing.test.ts @@ -117,6 +117,34 @@ function registerTwoTargets() { }; } +async function dispatchWebhookRequest(req: IncomingMessage) { + const res = createMockServerResponse(); + const handled = await handleGoogleChatWebhookRequest(req, res); + expect(handled).toBe(true); + return res; +} + +async function expectVerifiedRoute(params: { + request: IncomingMessage; + expectedStatus: number; + sinkA: ReturnType; + sinkB: ReturnType; + expectedSink: "none" | "A" | "B"; +}) { + const res = await dispatchWebhookRequest(params.request); + expect(res.statusCode).toBe(params.expectedStatus); + const expectedCounts = + params.expectedSink === "A" ? [1, 0] : params.expectedSink === "B" ? [0, 1] : [0, 0]; + expect(params.sinkA).toHaveBeenCalledTimes(expectedCounts[0]); + expect(params.sinkB).toHaveBeenCalledTimes(expectedCounts[1]); +} + +function mockSecondVerifierSuccess() { + vi.mocked(verifyGoogleChatRequest) + .mockResolvedValueOnce({ ok: false, reason: "invalid" }) + .mockResolvedValueOnce({ ok: true }); +} + describe("Google Chat webhook routing", () => { afterEach(() => { setActivePluginRegistry(createEmptyPluginRegistry()); @@ -165,45 +193,37 @@ describe("Google Chat webhook routing", () => { const { sinkA, sinkB, unregister } = registerTwoTargets(); try { - const res = createMockServerResponse(); - const handled = await handleGoogleChatWebhookRequest( - createWebhookRequest({ + await expectVerifiedRoute({ + request: createWebhookRequest({ authorization: "Bearer test-token", payload: { type: "ADDED_TO_SPACE", space: { name: "spaces/AAA" } }, }), - res, - ); - - expect(handled).toBe(true); - expect(res.statusCode).toBe(401); - expect(sinkA).not.toHaveBeenCalled(); - expect(sinkB).not.toHaveBeenCalled(); + expectedStatus: 401, + sinkA, + sinkB, + expectedSink: "none", + }); } finally { unregister(); } }); it("routes to the single verified target when earlier targets fail verification", async () => { - vi.mocked(verifyGoogleChatRequest) - .mockResolvedValueOnce({ ok: false, reason: "invalid" }) - .mockResolvedValueOnce({ ok: true }); + mockSecondVerifierSuccess(); const { sinkA, sinkB, unregister } = registerTwoTargets(); try { - const res = createMockServerResponse(); - const handled = await handleGoogleChatWebhookRequest( - createWebhookRequest({ + await expectVerifiedRoute({ + request: createWebhookRequest({ authorization: "Bearer test-token", payload: { type: "ADDED_TO_SPACE", space: { name: "spaces/BBB" } }, }), - res, - ); - - expect(handled).toBe(true); - expect(res.statusCode).toBe(200); - expect(sinkA).not.toHaveBeenCalled(); - expect(sinkB).toHaveBeenCalledTimes(1); + expectedStatus: 200, + sinkA, + sinkB, + expectedSink: "B", + }); } finally { unregister(); } @@ -218,10 +238,7 @@ describe("Google Chat webhook routing", () => { authorization: "Bearer invalid-token", }); const onSpy = vi.spyOn(req, "on"); - const res = createMockServerResponse(); - const handled = await handleGoogleChatWebhookRequest(req, res); - - expect(handled).toBe(true); + const res = await dispatchWebhookRequest(req); expect(res.statusCode).toBe(401); expect(onSpy).not.toHaveBeenCalledWith("data", expect.any(Function)); } finally { @@ -230,15 +247,12 @@ describe("Google Chat webhook routing", () => { }); it("supports add-on requests that provide systemIdToken in the body", async () => { - vi.mocked(verifyGoogleChatRequest) - .mockResolvedValueOnce({ ok: false, reason: "invalid" }) - .mockResolvedValueOnce({ ok: true }); + mockSecondVerifierSuccess(); const { sinkA, sinkB, unregister } = registerTwoTargets(); try { - const res = createMockServerResponse(); - const handled = await handleGoogleChatWebhookRequest( - createWebhookRequest({ + await expectVerifiedRoute({ + request: createWebhookRequest({ payload: { commonEventObject: { hostApp: "CHAT" }, authorizationEventObject: { systemIdToken: "addon-token" }, @@ -252,13 +266,11 @@ describe("Google Chat webhook routing", () => { }, }, }), - res, - ); - - expect(handled).toBe(true); - expect(res.statusCode).toBe(200); - expect(sinkA).not.toHaveBeenCalled(); - expect(sinkB).toHaveBeenCalledTimes(1); + expectedStatus: 200, + sinkA, + sinkB, + expectedSink: "B", + }); } finally { unregister(); } diff --git a/extensions/lobster/src/windows-spawn.test.ts b/extensions/lobster/src/windows-spawn.test.ts index e3d791e36e4..48e6ddc9a54 100644 --- a/extensions/lobster/src/windows-spawn.test.ts +++ b/extensions/lobster/src/windows-spawn.test.ts @@ -14,6 +14,19 @@ describe("resolveWindowsLobsterSpawn", () => { let tempDir = ""; const originalProcessState = snapshotPlatformPathEnv(); + async function expectUnwrappedShim(params: { + scriptPath: string; + shimPath: string; + shimLine: string; + }) { + await createWindowsCmdShimFixture(params); + + const target = resolveWindowsLobsterSpawn(params.shimPath, ["run", "noop"], process.env); + expect(target.command).toBe(process.execPath); + expect(target.argv).toEqual([params.scriptPath, "run", "noop"]); + expect(target.windowsHide).toBe(true); + } + beforeEach(async () => { tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-lobster-win-spawn-")); setProcessPlatform("win32"); @@ -30,31 +43,21 @@ describe("resolveWindowsLobsterSpawn", () => { it("unwraps cmd shim with %dp0% token", async () => { const scriptPath = path.join(tempDir, "shim-dist", "lobster-cli.cjs"); const shimPath = path.join(tempDir, "shim", "lobster.cmd"); - await createWindowsCmdShimFixture({ + await expectUnwrappedShim({ shimPath, scriptPath, shimLine: `"%dp0%\\..\\shim-dist\\lobster-cli.cjs" %*`, }); - - const target = resolveWindowsLobsterSpawn(shimPath, ["run", "noop"], process.env); - expect(target.command).toBe(process.execPath); - expect(target.argv).toEqual([scriptPath, "run", "noop"]); - expect(target.windowsHide).toBe(true); }); it("unwraps cmd shim with %~dp0% token", async () => { const scriptPath = path.join(tempDir, "shim-dist", "lobster-cli.cjs"); const shimPath = path.join(tempDir, "shim", "lobster.cmd"); - await createWindowsCmdShimFixture({ + await expectUnwrappedShim({ shimPath, scriptPath, shimLine: `"%~dp0%\\..\\shim-dist\\lobster-cli.cjs" %*`, }); - - const target = resolveWindowsLobsterSpawn(shimPath, ["run", "noop"], process.env); - expect(target.command).toBe(process.execPath); - expect(target.argv).toEqual([scriptPath, "run", "noop"]); - expect(target.windowsHide).toBe(true); }); it("ignores node.exe shim entries and picks lobster script", async () => { diff --git a/extensions/matrix/src/matrix/monitor/direct.test.ts b/extensions/matrix/src/matrix/monitor/direct.test.ts index 298b3996837..6688f76e649 100644 --- a/extensions/matrix/src/matrix/monitor/direct.test.ts +++ b/extensions/matrix/src/matrix/monitor/direct.test.ts @@ -7,6 +7,8 @@ import { createDirectRoomTracker } from "./direct.js"; type StateEvent = Record; type DmMap = Record; +const brokenDmRoomId = "!broken-dm:example.org"; +const defaultBrokenDmMembers = ["@alice:example.org", "@bot:example.org"]; function createMockClient(opts: { dmRooms?: DmMap; @@ -50,6 +52,21 @@ function createMockClient(opts: { }; } +function createBrokenDmClient(roomNameEvent?: StateEvent) { + return createMockClient({ + dmRooms: {}, + membersByRoom: { + [brokenDmRoomId]: defaultBrokenDmMembers, + }, + stateEvents: { + // is_direct not set on either member (e.g. Continuwuity bug) + [`${brokenDmRoomId}|m.room.member|@alice:example.org`]: {}, + [`${brokenDmRoomId}|m.room.member|@bot:example.org`]: {}, + ...(roomNameEvent ? { [`${brokenDmRoomId}|m.room.name|`]: roomNameEvent } : {}), + }, + }); +} + // --------------------------------------------------------------------------- // Tests -- isDirectMessage // --------------------------------------------------------------------------- @@ -131,22 +148,11 @@ describe("createDirectRoomTracker", () => { describe("conservative fallback (memberCount + room name)", () => { it("returns true for 2-member room WITHOUT a room name (broken flags)", async () => { - const client = createMockClient({ - dmRooms: {}, - membersByRoom: { - "!broken-dm:example.org": ["@alice:example.org", "@bot:example.org"], - }, - stateEvents: { - // is_direct not set on either member (e.g. Continuwuity bug) - "!broken-dm:example.org|m.room.member|@alice:example.org": {}, - "!broken-dm:example.org|m.room.member|@bot:example.org": {}, - // No m.room.name -> getRoomStateEvent will throw (event not found) - }, - }); + const client = createBrokenDmClient(); const tracker = createDirectRoomTracker(client as never); const result = await tracker.isDirectMessage({ - roomId: "!broken-dm:example.org", + roomId: brokenDmRoomId, senderId: "@alice:example.org", }); @@ -154,21 +160,11 @@ describe("createDirectRoomTracker", () => { }); it("returns true for 2-member room with empty room name", async () => { - const client = createMockClient({ - dmRooms: {}, - membersByRoom: { - "!broken-dm:example.org": ["@alice:example.org", "@bot:example.org"], - }, - stateEvents: { - "!broken-dm:example.org|m.room.member|@alice:example.org": {}, - "!broken-dm:example.org|m.room.member|@bot:example.org": {}, - "!broken-dm:example.org|m.room.name|": { name: "" }, - }, - }); + const client = createBrokenDmClient({ name: "" }); const tracker = createDirectRoomTracker(client as never); const result = await tracker.isDirectMessage({ - roomId: "!broken-dm:example.org", + roomId: brokenDmRoomId, senderId: "@alice:example.org", }); diff --git a/extensions/matrix/src/matrix/monitor/events.test.ts b/extensions/matrix/src/matrix/monitor/events.test.ts index 9179cf69ee3..3c08a0230d1 100644 --- a/extensions/matrix/src/matrix/monitor/events.test.ts +++ b/extensions/matrix/src/matrix/monitor/events.test.ts @@ -12,6 +12,8 @@ vi.mock("../send.js", () => ({ })); describe("registerMatrixMonitorEvents", () => { + const roomId = "!room:example.org"; + beforeEach(() => { sendReadReceiptMatrixMock.mockClear(); }); @@ -53,6 +55,16 @@ describe("registerMatrixMonitorEvents", () => { return { client, getUserId, onRoomMessage, roomMessageHandler, logVerboseMessage }; } + async function expectForwardedWithoutReadReceipt(event: MatrixRawEvent) { + const { onRoomMessage, roomMessageHandler } = createHarness(); + + roomMessageHandler(roomId, event); + await vi.waitFor(() => { + expect(onRoomMessage).toHaveBeenCalledWith(roomId, event); + }); + expect(sendReadReceiptMatrixMock).not.toHaveBeenCalled(); + } + it("sends read receipt immediately for non-self messages", async () => { const { client, onRoomMessage, roomMessageHandler } = createHarness(); const event = { @@ -69,30 +81,16 @@ describe("registerMatrixMonitorEvents", () => { }); it("does not send read receipts for self messages", async () => { - const { onRoomMessage, roomMessageHandler } = createHarness(); - const event = { + await expectForwardedWithoutReadReceipt({ event_id: "$e2", sender: "@bot:example.org", - } as MatrixRawEvent; - - roomMessageHandler("!room:example.org", event); - await vi.waitFor(() => { - expect(onRoomMessage).toHaveBeenCalledWith("!room:example.org", event); }); - expect(sendReadReceiptMatrixMock).not.toHaveBeenCalled(); }); it("skips receipt when message lacks sender or event id", async () => { - const { onRoomMessage, roomMessageHandler } = createHarness(); - const event = { + await expectForwardedWithoutReadReceipt({ sender: "@alice:example.org", - } as MatrixRawEvent; - - roomMessageHandler("!room:example.org", event); - await vi.waitFor(() => { - expect(onRoomMessage).toHaveBeenCalledWith("!room:example.org", event); }); - expect(sendReadReceiptMatrixMock).not.toHaveBeenCalled(); }); it("caches self user id across messages", async () => { diff --git a/extensions/mattermost/src/mattermost/monitor-helpers.ts b/extensions/mattermost/src/mattermost/monitor-helpers.ts index de264e6cf2c..219c0562638 100644 --- a/extensions/mattermost/src/mattermost/monitor-helpers.ts +++ b/extensions/mattermost/src/mattermost/monitor-helpers.ts @@ -41,12 +41,12 @@ function normalizeAgentId(value: string | undefined | null): string { type AgentEntry = NonNullable["list"]>[number]; +function isAgentEntry(entry: unknown): entry is AgentEntry { + return Boolean(entry && typeof entry === "object"); +} + function listAgents(cfg: OpenClawConfig): AgentEntry[] { - const list = cfg.agents?.list; - if (!Array.isArray(list)) { - return []; - } - return list.filter((entry): entry is AgentEntry => Boolean(entry && typeof entry === "object")); + return Array.isArray(cfg.agents?.list) ? cfg.agents.list.filter(isAgentEntry) : []; } function resolveAgentEntry(cfg: OpenClawConfig, agentId: string): AgentEntry | undefined { diff --git a/extensions/msteams/src/attachments/shared.test.ts b/extensions/msteams/src/attachments/shared.test.ts index 186a70f71aa..3e29e65aac4 100644 --- a/extensions/msteams/src/attachments/shared.test.ts +++ b/extensions/msteams/src/attachments/shared.test.ts @@ -31,6 +31,23 @@ function mockFetchWithRedirect(redirectMap: Record, finalBody = }); } +async function expectSafeFetchStatus(params: { + fetchMock: ReturnType; + url: string; + allowHosts: string[]; + expectedStatus: number; + resolveFn?: typeof publicResolve; +}) { + const res = await safeFetch({ + url: params.url, + allowHosts: params.allowHosts, + fetchFn: params.fetchMock as unknown as typeof fetch, + resolveFn: params.resolveFn ?? publicResolve, + }); + expect(res.status).toBe(params.expectedStatus); + return res; +} + describe("msteams attachment allowlists", () => { it("normalizes wildcard host lists", () => { expect(resolveAllowedHosts(["*", "graph.microsoft.com"])).toEqual(["*"]); @@ -121,13 +138,12 @@ describe("safeFetch", () => { const fetchMock = vi.fn(async (_url: string, _init?: RequestInit) => { return new Response("ok", { status: 200 }); }); - const res = await safeFetch({ + await expectSafeFetchStatus({ + fetchMock, url: "https://teams.sharepoint.com/file.pdf", allowHosts: ["sharepoint.com"], - fetchFn: fetchMock as unknown as typeof fetch, - resolveFn: publicResolve, + expectedStatus: 200, }); - expect(res.status).toBe(200); expect(fetchMock).toHaveBeenCalledOnce(); // Should have used redirect: "manual" expect(fetchMock.mock.calls[0][1]).toHaveProperty("redirect", "manual"); @@ -137,13 +153,12 @@ describe("safeFetch", () => { const fetchMock = mockFetchWithRedirect({ "https://teams.sharepoint.com/file.pdf": "https://cdn.sharepoint.com/storage/file.pdf", }); - const res = await safeFetch({ + await expectSafeFetchStatus({ + fetchMock, url: "https://teams.sharepoint.com/file.pdf", allowHosts: ["sharepoint.com"], - fetchFn: fetchMock as unknown as typeof fetch, - resolveFn: publicResolve, + expectedStatus: 200, }); - expect(res.status).toBe(200); expect(fetchMock).toHaveBeenCalledTimes(2); }); diff --git a/extensions/slack/src/channel.test.ts b/extensions/slack/src/channel.test.ts index b846d6e3cd7..98fbddca77d 100644 --- a/extensions/slack/src/channel.test.ts +++ b/extensions/slack/src/channel.test.ts @@ -15,6 +15,18 @@ vi.mock("./runtime.js", () => ({ import { slackPlugin } from "./channel.js"; +async function getSlackConfiguredState(cfg: OpenClawConfig) { + const account = slackPlugin.config.resolveAccount(cfg, "default"); + return { + configured: slackPlugin.config.isConfigured?.(account, cfg), + snapshot: await slackPlugin.status?.buildAccountSnapshot?.({ + account, + cfg, + runtime: undefined, + }), + }; +} + describe("slackPlugin actions", () => { it("prefers session lookup for announce target routing", () => { expect(slackPlugin.meta.preferSessionLookupForAnnounceTarget).toBe(true); @@ -189,13 +201,7 @@ describe("slackPlugin config", () => { }, }; - const account = slackPlugin.config.resolveAccount(cfg, "default"); - const configured = slackPlugin.config.isConfigured?.(account, cfg); - const snapshot = await slackPlugin.status?.buildAccountSnapshot?.({ - account, - cfg, - runtime: undefined, - }); + const { configured, snapshot } = await getSlackConfiguredState(cfg); expect(configured).toBe(true); expect(snapshot?.configured).toBe(true); @@ -211,13 +217,7 @@ describe("slackPlugin config", () => { }, }; - const account = slackPlugin.config.resolveAccount(cfg, "default"); - const configured = slackPlugin.config.isConfigured?.(account, cfg); - const snapshot = await slackPlugin.status?.buildAccountSnapshot?.({ - account, - cfg, - runtime: undefined, - }); + const { configured, snapshot } = await getSlackConfiguredState(cfg); expect(configured).toBe(false); expect(snapshot?.configured).toBe(false); diff --git a/extensions/synology-chat/src/types.ts b/extensions/synology-chat/src/types.ts index 7ba222531c6..842c2ee97bb 100644 --- a/extensions/synology-chat/src/types.ts +++ b/extensions/synology-chat/src/types.ts @@ -2,8 +2,7 @@ * Type definitions for the Synology Chat channel plugin. */ -/** Raw channel config from openclaw.json channels.synology-chat */ -export interface SynologyChatChannelConfig { +type SynologyChatConfigFields = { enabled?: boolean; token?: string; incomingUrl?: string; @@ -14,22 +13,15 @@ export interface SynologyChatChannelConfig { rateLimitPerMinute?: number; botName?: string; allowInsecureSsl?: boolean; +}; + +/** Raw channel config from openclaw.json channels.synology-chat */ +export interface SynologyChatChannelConfig extends SynologyChatConfigFields { accounts?: Record; } /** Raw per-account config (overrides base config) */ -export interface SynologyChatAccountRaw { - enabled?: boolean; - token?: string; - incomingUrl?: string; - nasHost?: string; - webhookPath?: string; - dmPolicy?: "open" | "allowlist" | "disabled"; - allowedUserIds?: string | string[]; - rateLimitPerMinute?: number; - botName?: string; - allowInsecureSsl?: boolean; -} +export interface SynologyChatAccountRaw extends SynologyChatConfigFields {} /** Fully resolved account config with defaults applied */ export interface ResolvedSynologyChatAccount { diff --git a/extensions/telegram/src/channel.test.ts b/extensions/telegram/src/channel.test.ts index f0736069015..a957a3e5b1c 100644 --- a/extensions/telegram/src/channel.test.ts +++ b/extensions/telegram/src/channel.test.ts @@ -91,6 +91,30 @@ function installGatewayRuntime(params?: { probeOk?: boolean; botUsername?: strin }; } +function configureOpsProxyNetwork(cfg: OpenClawConfig) { + cfg.channels!.telegram!.accounts!.ops = { + ...cfg.channels!.telegram!.accounts!.ops, + proxy: "http://127.0.0.1:8888", + network: { + autoSelectFamily: false, + dnsResultOrder: "ipv4first", + }, + }; +} + +function installSendMessageRuntime( + sendMessageTelegram: ReturnType, +): ReturnType { + setTelegramRuntime({ + channel: { + telegram: { + sendMessageTelegram, + }, + }, + } as unknown as PluginRuntime); + return sendMessageTelegram; +} + describe("telegramPlugin duplicate token guard", () => { it("marks secondary account as not configured when token is shared", async () => { const cfg = createCfg(); @@ -176,14 +200,7 @@ describe("telegramPlugin duplicate token guard", () => { }); const cfg = createCfg(); - cfg.channels!.telegram!.accounts!.ops = { - ...cfg.channels!.telegram!.accounts!.ops, - proxy: "http://127.0.0.1:8888", - network: { - autoSelectFamily: false, - dnsResultOrder: "ipv4first", - }, - }; + configureOpsProxyNetwork(cfg); const account = telegramPlugin.config.resolveAccount(cfg, "ops"); await telegramPlugin.status!.probeAccount!({ @@ -215,13 +232,9 @@ describe("telegramPlugin duplicate token guard", () => { }); const cfg = createCfg(); + configureOpsProxyNetwork(cfg); cfg.channels!.telegram!.accounts!.ops = { ...cfg.channels!.telegram!.accounts!.ops, - proxy: "http://127.0.0.1:8888", - network: { - autoSelectFamily: false, - dnsResultOrder: "ipv4first", - }, groups: { "-100123": { requireMention: false }, }, @@ -249,14 +262,9 @@ describe("telegramPlugin duplicate token guard", () => { }); it("forwards mediaLocalRoots to sendMessageTelegram for outbound media sends", async () => { - const sendMessageTelegram = vi.fn(async () => ({ messageId: "tg-1" })); - setTelegramRuntime({ - channel: { - telegram: { - sendMessageTelegram, - }, - }, - } as unknown as PluginRuntime); + const sendMessageTelegram = installSendMessageRuntime( + vi.fn(async () => ({ messageId: "tg-1" })), + ); const result = await telegramPlugin.outbound!.sendMedia!({ cfg: createCfg(), @@ -279,14 +287,9 @@ describe("telegramPlugin duplicate token guard", () => { }); it("preserves buttons for outbound text payload sends", async () => { - const sendMessageTelegram = vi.fn(async () => ({ messageId: "tg-2" })); - setTelegramRuntime({ - channel: { - telegram: { - sendMessageTelegram, - }, - }, - } as unknown as PluginRuntime); + const sendMessageTelegram = installSendMessageRuntime( + vi.fn(async () => ({ messageId: "tg-2" })), + ); const result = await telegramPlugin.outbound!.sendPayload!({ cfg: createCfg(), @@ -314,17 +317,12 @@ describe("telegramPlugin duplicate token guard", () => { }); it("sends outbound payload media lists and keeps buttons on the first message only", async () => { - const sendMessageTelegram = vi - .fn() - .mockResolvedValueOnce({ messageId: "tg-3", chatId: "12345" }) - .mockResolvedValueOnce({ messageId: "tg-4", chatId: "12345" }); - setTelegramRuntime({ - channel: { - telegram: { - sendMessageTelegram, - }, - }, - } as unknown as PluginRuntime); + const sendMessageTelegram = installSendMessageRuntime( + vi + .fn() + .mockResolvedValueOnce({ messageId: "tg-3", chatId: "12345" }) + .mockResolvedValueOnce({ messageId: "tg-4", chatId: "12345" }), + ); const result = await telegramPlugin.outbound!.sendPayload!({ cfg: createCfg(), diff --git a/extensions/twitch/src/outbound.test.ts b/extensions/twitch/src/outbound.test.ts index 7b480df32dd..f58e2d1ad48 100644 --- a/extensions/twitch/src/outbound.test.ts +++ b/extensions/twitch/src/outbound.test.ts @@ -46,6 +46,20 @@ function assertResolvedTarget( return result.to; } +function expectTargetError( + resolveTarget: NonNullable, + params: Parameters>[0], + expectedMessage: string, +) { + const result = resolveTarget(params); + + expect(result.ok).toBe(false); + if (result.ok) { + throw new Error("expected resolveTarget to fail"); + } + expect(result.error.message).toContain(expectedMessage); +} + describe("outbound", () => { const mockAccount = { ...BASE_TWITCH_TEST_ACCOUNT, @@ -106,17 +120,15 @@ describe("outbound", () => { }); it("should error when target not in allowlist (implicit mode)", () => { - const result = resolveTarget({ - to: "#notallowed", - mode: "implicit", - allowFrom: ["#primary", "#secondary"], - }); - - expect(result.ok).toBe(false); - if (result.ok) { - throw new Error("expected resolveTarget to fail"); - } - expect(result.error.message).toContain("Twitch"); + expectTargetError( + resolveTarget, + { + to: "#notallowed", + mode: "implicit", + allowFrom: ["#primary", "#secondary"], + }, + "Twitch", + ); }); it("should accept any target when allowlist is empty", () => { @@ -131,59 +143,51 @@ describe("outbound", () => { }); it("should error when no target provided with allowlist", () => { - const result = resolveTarget({ - to: undefined, - mode: "implicit", - allowFrom: ["#fallback", "#other"], - }); - - expect(result.ok).toBe(false); - if (result.ok) { - throw new Error("expected resolveTarget to fail"); - } - expect(result.error.message).toContain("Twitch"); + expectTargetError( + resolveTarget, + { + to: undefined, + mode: "implicit", + allowFrom: ["#fallback", "#other"], + }, + "Twitch", + ); }); it("should return error when no target and no allowlist", () => { - const result = resolveTarget({ - to: undefined, - mode: "explicit", - allowFrom: [], - }); - - expect(result.ok).toBe(false); - if (result.ok) { - throw new Error("expected resolveTarget to fail"); - } - expect(result.error.message).toContain("Missing target"); + expectTargetError( + resolveTarget, + { + to: undefined, + mode: "explicit", + allowFrom: [], + }, + "Missing target", + ); }); it("should handle whitespace-only target", () => { - const result = resolveTarget({ - to: " ", - mode: "explicit", - allowFrom: [], - }); - - expect(result.ok).toBe(false); - if (result.ok) { - throw new Error("expected resolveTarget to fail"); - } - expect(result.error.message).toContain("Missing target"); + expectTargetError( + resolveTarget, + { + to: " ", + mode: "explicit", + allowFrom: [], + }, + "Missing target", + ); }); it("should error when target normalizes to empty string", () => { - const result = resolveTarget({ - to: "#", - mode: "explicit", - allowFrom: [], - }); - - expect(result.ok).toBe(false); - if (result.ok) { - throw new Error("expected resolveTarget to fail"); - } - expect(result.error.message).toContain("Twitch"); + expectTargetError( + resolveTarget, + { + to: "#", + mode: "explicit", + allowFrom: [], + }, + "Twitch", + ); }); it("should filter wildcard from allowlist when checking membership", () => { diff --git a/extensions/twitch/src/send.test.ts b/extensions/twitch/src/send.test.ts index e7185b3f5fb..b45321229a4 100644 --- a/extensions/twitch/src/send.test.ts +++ b/extensions/twitch/src/send.test.ts @@ -55,7 +55,10 @@ describe("send", () => { installTwitchTestHooks(); describe("sendMessageTwitchInternal", () => { - it("should send a message successfully", async () => { + async function mockSuccessfulSend(params: { + messageId: string; + stripMarkdown?: (text: string) => string; + }) { const { getAccountConfig } = await import("./config.js"); const { getClientManager } = await import("./client-manager-registry.js"); const { stripMarkdownForTwitch } = await import("./utils/markdown.js"); @@ -64,10 +67,18 @@ describe("send", () => { vi.mocked(getClientManager).mockReturnValue({ sendMessage: vi.fn().mockResolvedValue({ ok: true, - messageId: "twitch-msg-123", + messageId: params.messageId, }), } as unknown as ReturnType); - vi.mocked(stripMarkdownForTwitch).mockImplementation((text) => text); + vi.mocked(stripMarkdownForTwitch).mockImplementation( + params.stripMarkdown ?? ((text) => text), + ); + + return { stripMarkdownForTwitch }; + } + + it("should send a message successfully", async () => { + await mockSuccessfulSend({ messageId: "twitch-msg-123" }); const result = await sendMessageTwitchInternal( "#testchannel", @@ -83,18 +94,10 @@ describe("send", () => { }); it("should strip markdown when enabled", async () => { - const { getAccountConfig } = await import("./config.js"); - const { getClientManager } = await import("./client-manager-registry.js"); - const { stripMarkdownForTwitch } = await import("./utils/markdown.js"); - - vi.mocked(getAccountConfig).mockReturnValue(mockAccount); - vi.mocked(getClientManager).mockReturnValue({ - sendMessage: vi.fn().mockResolvedValue({ - ok: true, - messageId: "twitch-msg-456", - }), - } as unknown as ReturnType); - vi.mocked(stripMarkdownForTwitch).mockImplementation((text) => text.replace(/\*\*/g, "")); + const { stripMarkdownForTwitch } = await mockSuccessfulSend({ + messageId: "twitch-msg-456", + stripMarkdown: (text) => text.replace(/\*\*/g, ""), + }); await sendMessageTwitchInternal( "#testchannel", diff --git a/extensions/voice-call/src/providers/telnyx.test.ts b/extensions/voice-call/src/providers/telnyx.test.ts index c083070229f..15a4cc8f17f 100644 --- a/extensions/voice-call/src/providers/telnyx.test.ts +++ b/extensions/voice-call/src/providers/telnyx.test.ts @@ -22,6 +22,34 @@ function decodeBase64Url(input: string): Buffer { return Buffer.from(padded, "base64"); } +function createSignedTelnyxCtx(params: { + privateKey: crypto.KeyObject; + rawBody: string; +}): WebhookContext { + const timestamp = String(Math.floor(Date.now() / 1000)); + const signedPayload = `${timestamp}|${params.rawBody}`; + const signature = crypto + .sign(null, Buffer.from(signedPayload), params.privateKey) + .toString("base64"); + + return createCtx({ + rawBody: params.rawBody, + headers: { + "telnyx-signature-ed25519": signature, + "telnyx-timestamp": timestamp, + }, + }); +} + +function expectReplayVerification( + results: Array<{ ok: boolean; isReplay?: boolean; verifiedRequestKey?: string }>, +) { + expect(results.map((result) => result.ok)).toEqual([true, true]); + expect(results.map((result) => Boolean(result.isReplay))).toEqual([false, true]); + expect(results[0]?.verifiedRequestKey).toEqual(expect.any(String)); + expect(results[1]?.verifiedRequestKey).toBe(results[0]?.verifiedRequestKey); +} + function expectWebhookVerificationSucceeds(params: { publicKey: string; privateKey: crypto.KeyObject; @@ -35,20 +63,8 @@ function expectWebhookVerificationSucceeds(params: { event_type: "call.initiated", payload: { call_control_id: "x" }, }); - const timestamp = String(Math.floor(Date.now() / 1000)); - const signedPayload = `${timestamp}|${rawBody}`; - const signature = crypto - .sign(null, Buffer.from(signedPayload), params.privateKey) - .toString("base64"); - const result = provider.verifyWebhook( - createCtx({ - rawBody, - headers: { - "telnyx-signature-ed25519": signature, - "telnyx-timestamp": timestamp, - }, - }), + createSignedTelnyxCtx({ privateKey: params.privateKey, rawBody }), ); expect(result.ok).toBe(true); } @@ -117,26 +133,12 @@ describe("TelnyxProvider.verifyWebhook", () => { payload: { call_control_id: "call-replay-test" }, nonce: crypto.randomUUID(), }); - const timestamp = String(Math.floor(Date.now() / 1000)); - const signedPayload = `${timestamp}|${rawBody}`; - const signature = crypto.sign(null, Buffer.from(signedPayload), privateKey).toString("base64"); - const ctx = createCtx({ - rawBody, - headers: { - "telnyx-signature-ed25519": signature, - "telnyx-timestamp": timestamp, - }, - }); + const ctx = createSignedTelnyxCtx({ privateKey, rawBody }); const first = provider.verifyWebhook(ctx); const second = provider.verifyWebhook(ctx); - expect(first.ok).toBe(true); - expect(first.isReplay).toBeFalsy(); - expect(first.verifiedRequestKey).toBeTruthy(); - expect(second.ok).toBe(true); - expect(second.isReplay).toBe(true); - expect(second.verifiedRequestKey).toBe(first.verifiedRequestKey); + expectReplayVerification([first, second]); }); }); diff --git a/extensions/voice-call/src/webhook-security.test.ts b/extensions/voice-call/src/webhook-security.test.ts index 3134f18b729..3fe3cd473a1 100644 --- a/extensions/voice-call/src/webhook-security.test.ts +++ b/extensions/voice-call/src/webhook-security.test.ts @@ -98,6 +98,51 @@ function expectReplayResultPair( expect(second.verifiedRequestKey).toBe(first.verifiedRequestKey); } +function expectAcceptedWebhookVersion( + result: { ok: boolean; version?: string }, + version: "v2" | "v3", +) { + expect(result).toMatchObject({ ok: true, version }); +} + +function verifyTwilioNgrokLoopback(signature: string) { + return verifyTwilioWebhook( + { + headers: { + host: "127.0.0.1:3334", + "x-forwarded-proto": "https", + "x-forwarded-host": "local.ngrok-free.app", + "x-twilio-signature": signature, + }, + rawBody: "CallSid=CS123&CallStatus=completed&From=%2B15550000000", + url: "http://127.0.0.1:3334/voice/webhook", + method: "POST", + remoteAddress: "127.0.0.1", + }, + "test-auth-token", + { allowNgrokFreeTierLoopbackBypass: true }, + ); +} + +function verifyTwilioSignedRequest(params: { + headers: Record; + rawBody: string; + authToken: string; + publicUrl: string; +}) { + return verifyTwilioWebhook( + { + headers: params.headers, + rawBody: params.rawBody, + url: "http://local/voice/webhook?callId=abc", + method: "POST", + query: { callId: "abc" }, + }, + params.authToken, + { publicUrl: params.publicUrl }, + ); +} + describe("verifyPlivoWebhook", () => { it("accepts valid V2 signature", () => { const authToken = "test-auth-token"; @@ -127,8 +172,7 @@ describe("verifyPlivoWebhook", () => { authToken, ); - expect(result.ok).toBe(true); - expect(result.version).toBe("v2"); + expectAcceptedWebhookVersion(result, "v2"); }); it("accepts valid V3 signature (including multi-signature header)", () => { @@ -161,8 +205,7 @@ describe("verifyPlivoWebhook", () => { authToken, ); - expect(result.ok).toBe(true); - expect(result.version).toBe("v3"); + expectAcceptedWebhookVersion(result, "v3"); }); it("rejects missing signatures", () => { @@ -317,35 +360,10 @@ describe("verifyTwilioWebhook", () => { "i-twilio-idempotency-token": "idem-replay-1", }; - const first = verifyTwilioWebhook( - { - headers, - rawBody: postBody, - url: "http://local/voice/webhook?callId=abc", - method: "POST", - query: { callId: "abc" }, - }, - authToken, - { publicUrl }, - ); - const second = verifyTwilioWebhook( - { - headers, - rawBody: postBody, - url: "http://local/voice/webhook?callId=abc", - method: "POST", - query: { callId: "abc" }, - }, - authToken, - { publicUrl }, - ); + const first = verifyTwilioSignedRequest({ headers, rawBody: postBody, authToken, publicUrl }); + const second = verifyTwilioSignedRequest({ headers, rawBody: postBody, authToken, publicUrl }); - expect(first.ok).toBe(true); - expect(first.isReplay).toBeFalsy(); - expect(first.verifiedRequestKey).toBeTruthy(); - expect(second.ok).toBe(true); - expect(second.isReplay).toBe(true); - expect(second.verifiedRequestKey).toBe(first.verifiedRequestKey); + expectReplayResultPair(first, second); }); it("treats changed idempotency header as replay for identical signed requests", () => { @@ -355,45 +373,30 @@ describe("verifyTwilioWebhook", () => { const postBody = "CallSid=CS778&CallStatus=completed&From=%2B15550000000"; const signature = twilioSignature({ authToken, url: urlWithQuery, postBody }); - const first = verifyTwilioWebhook( - { - headers: { - host: "example.com", - "x-forwarded-proto": "https", - "x-twilio-signature": signature, - "i-twilio-idempotency-token": "idem-replay-a", - }, - rawBody: postBody, - url: "http://local/voice/webhook?callId=abc", - method: "POST", - query: { callId: "abc" }, + const first = verifyTwilioSignedRequest({ + headers: { + host: "example.com", + "x-forwarded-proto": "https", + "x-twilio-signature": signature, + "i-twilio-idempotency-token": "idem-replay-a", }, + rawBody: postBody, authToken, - { publicUrl }, - ); - const second = verifyTwilioWebhook( - { - headers: { - host: "example.com", - "x-forwarded-proto": "https", - "x-twilio-signature": signature, - "i-twilio-idempotency-token": "idem-replay-b", - }, - rawBody: postBody, - url: "http://local/voice/webhook?callId=abc", - method: "POST", - query: { callId: "abc" }, + publicUrl, + }); + const second = verifyTwilioSignedRequest({ + headers: { + host: "example.com", + "x-forwarded-proto": "https", + "x-twilio-signature": signature, + "i-twilio-idempotency-token": "idem-replay-b", }, + rawBody: postBody, authToken, - { publicUrl }, - ); + publicUrl, + }); - expect(first.ok).toBe(true); - expect(first.isReplay).toBe(false); - expect(first.verifiedRequestKey).toBeTruthy(); - expect(second.ok).toBe(true); - expect(second.isReplay).toBe(true); - expect(second.verifiedRequestKey).toBe(first.verifiedRequestKey); + expectReplayResultPair(first, second); }); it("rejects invalid signatures even when attacker injects forwarded host", () => { @@ -422,57 +425,22 @@ describe("verifyTwilioWebhook", () => { }); it("accepts valid signatures for ngrok free tier on loopback when compatibility mode is enabled", () => { - const authToken = "test-auth-token"; - const postBody = "CallSid=CS123&CallStatus=completed&From=%2B15550000000"; const webhookUrl = "https://local.ngrok-free.app/voice/webhook"; const signature = twilioSignature({ - authToken, + authToken: "test-auth-token", url: webhookUrl, - postBody, + postBody: "CallSid=CS123&CallStatus=completed&From=%2B15550000000", }); - const result = verifyTwilioWebhook( - { - headers: { - host: "127.0.0.1:3334", - "x-forwarded-proto": "https", - "x-forwarded-host": "local.ngrok-free.app", - "x-twilio-signature": signature, - }, - rawBody: postBody, - url: "http://127.0.0.1:3334/voice/webhook", - method: "POST", - remoteAddress: "127.0.0.1", - }, - authToken, - { allowNgrokFreeTierLoopbackBypass: true }, - ); + const result = verifyTwilioNgrokLoopback(signature); expect(result.ok).toBe(true); expect(result.verificationUrl).toBe(webhookUrl); }); it("does not allow invalid signatures for ngrok free tier on loopback", () => { - const authToken = "test-auth-token"; - const postBody = "CallSid=CS123&CallStatus=completed&From=%2B15550000000"; - - const result = verifyTwilioWebhook( - { - headers: { - host: "127.0.0.1:3334", - "x-forwarded-proto": "https", - "x-forwarded-host": "local.ngrok-free.app", - "x-twilio-signature": "invalid", - }, - rawBody: postBody, - url: "http://127.0.0.1:3334/voice/webhook", - method: "POST", - remoteAddress: "127.0.0.1", - }, - authToken, - { allowNgrokFreeTierLoopbackBypass: true }, - ); + const result = verifyTwilioNgrokLoopback("invalid"); expect(result.ok).toBe(false); expect(result.reason).toMatch(/Invalid signature/); diff --git a/extensions/voice-call/src/webhook.test.ts b/extensions/voice-call/src/webhook.test.ts index f5a827a3ef3..6297a69f14a 100644 --- a/extensions/voice-call/src/webhook.test.ts +++ b/extensions/voice-call/src/webhook.test.ts @@ -56,6 +56,28 @@ const createManager = (calls: CallRecord[]) => { return { manager, endCall, processEvent }; }; +async function runStaleCallReaperCase(params: { + callAgeMs: number; + staleCallReaperSeconds: number; + advanceMs: number; +}) { + const now = new Date("2026-02-16T00:00:00Z"); + vi.setSystemTime(now); + + const call = createCall(now.getTime() - params.callAgeMs); + const { manager, endCall } = createManager([call]); + const config = createConfig({ staleCallReaperSeconds: params.staleCallReaperSeconds }); + const server = new VoiceCallWebhookServer(config, manager, provider); + + try { + await server.start(); + await vi.advanceTimersByTimeAsync(params.advanceMs); + return { call, endCall }; + } finally { + await server.stop(); + } +} + async function postWebhookForm(server: VoiceCallWebhookServer, baseUrl: string, body: string) { const address = ( server as unknown as { server?: { address?: () => unknown } } @@ -81,39 +103,21 @@ describe("VoiceCallWebhookServer stale call reaper", () => { }); it("ends calls older than staleCallReaperSeconds", async () => { - const now = new Date("2026-02-16T00:00:00Z"); - vi.setSystemTime(now); - - const call = createCall(now.getTime() - 120_000); - const { manager, endCall } = createManager([call]); - const config = createConfig({ staleCallReaperSeconds: 60 }); - const server = new VoiceCallWebhookServer(config, manager, provider); - - try { - await server.start(); - await vi.advanceTimersByTimeAsync(30_000); - expect(endCall).toHaveBeenCalledWith(call.callId); - } finally { - await server.stop(); - } + const { call, endCall } = await runStaleCallReaperCase({ + callAgeMs: 120_000, + staleCallReaperSeconds: 60, + advanceMs: 30_000, + }); + expect(endCall).toHaveBeenCalledWith(call.callId); }); it("skips calls that are younger than the threshold", async () => { - const now = new Date("2026-02-16T00:00:00Z"); - vi.setSystemTime(now); - - const call = createCall(now.getTime() - 10_000); - const { manager, endCall } = createManager([call]); - const config = createConfig({ staleCallReaperSeconds: 60 }); - const server = new VoiceCallWebhookServer(config, manager, provider); - - try { - await server.start(); - await vi.advanceTimersByTimeAsync(30_000); - expect(endCall).not.toHaveBeenCalled(); - } finally { - await server.stop(); - } + const { endCall } = await runStaleCallReaperCase({ + callAgeMs: 10_000, + staleCallReaperSeconds: 60, + advanceMs: 30_000, + }); + expect(endCall).not.toHaveBeenCalled(); }); it("does not run when staleCallReaperSeconds is disabled", async () => { diff --git a/extensions/zalo/src/api.test.ts b/extensions/zalo/src/api.test.ts index 00198f5072e..ffdeab84ae4 100644 --- a/extensions/zalo/src/api.test.ts +++ b/extensions/zalo/src/api.test.ts @@ -1,31 +1,26 @@ import { describe, expect, it, vi } from "vitest"; import { deleteWebhook, getWebhookInfo, sendChatAction, type ZaloFetch } from "./api.js"; +function createOkFetcher() { + return vi.fn(async () => new Response(JSON.stringify({ ok: true, result: {} }))); +} + +async function expectPostJsonRequest(run: (token: string, fetcher: ZaloFetch) => Promise) { + const fetcher = createOkFetcher(); + await run("test-token", fetcher); + expect(fetcher).toHaveBeenCalledTimes(1); + const [, init] = fetcher.mock.calls[0] ?? []; + expect(init?.method).toBe("POST"); + expect(init?.headers).toEqual({ "Content-Type": "application/json" }); +} + describe("Zalo API request methods", () => { it("uses POST for getWebhookInfo", async () => { - const fetcher = vi.fn( - async () => new Response(JSON.stringify({ ok: true, result: {} })), - ); - - await getWebhookInfo("test-token", fetcher); - - expect(fetcher).toHaveBeenCalledTimes(1); - const [, init] = fetcher.mock.calls[0] ?? []; - expect(init?.method).toBe("POST"); - expect(init?.headers).toEqual({ "Content-Type": "application/json" }); + await expectPostJsonRequest(getWebhookInfo); }); it("keeps POST for deleteWebhook", async () => { - const fetcher = vi.fn( - async () => new Response(JSON.stringify({ ok: true, result: {} })), - ); - - await deleteWebhook("test-token", fetcher); - - expect(fetcher).toHaveBeenCalledTimes(1); - const [, init] = fetcher.mock.calls[0] ?? []; - expect(init?.method).toBe("POST"); - expect(init?.headers).toEqual({ "Content-Type": "application/json" }); + await expectPostJsonRequest(deleteWebhook); }); it("aborts sendChatAction when the typing timeout elapses", async () => { diff --git a/extensions/zalouser/src/accounts.ts b/extensions/zalouser/src/accounts.ts index 5ebec2d2c93..26a02ed47a0 100644 --- a/extensions/zalouser/src/accounts.ts +++ b/extensions/zalouser/src/accounts.ts @@ -43,17 +43,24 @@ function resolveProfile(config: ZalouserAccountConfig, accountId: string): strin return "default"; } -export async function resolveZalouserAccount(params: { - cfg: OpenClawConfig; - accountId?: string | null; -}): Promise { +function resolveZalouserAccountBase(params: { cfg: OpenClawConfig; accountId?: string | null }) { const accountId = normalizeAccountId(params.accountId); const baseEnabled = (params.cfg.channels?.zalouser as ZalouserConfig | undefined)?.enabled !== false; const merged = mergeZalouserAccountConfig(params.cfg, accountId); - const accountEnabled = merged.enabled !== false; - const enabled = baseEnabled && accountEnabled; - const profile = resolveProfile(merged, accountId); + return { + accountId, + enabled: baseEnabled && merged.enabled !== false, + merged, + profile: resolveProfile(merged, accountId), + }; +} + +export async function resolveZalouserAccount(params: { + cfg: OpenClawConfig; + accountId?: string | null; +}): Promise { + const { accountId, enabled, merged, profile } = resolveZalouserAccountBase(params); const authenticated = await checkZaloAuthenticated(profile); return { @@ -70,13 +77,7 @@ export function resolveZalouserAccountSync(params: { cfg: OpenClawConfig; accountId?: string | null; }): ResolvedZalouserAccount { - const accountId = normalizeAccountId(params.accountId); - const baseEnabled = - (params.cfg.channels?.zalouser as ZalouserConfig | undefined)?.enabled !== false; - const merged = mergeZalouserAccountConfig(params.cfg, accountId); - const accountEnabled = merged.enabled !== false; - const enabled = baseEnabled && accountEnabled; - const profile = resolveProfile(merged, accountId); + const { accountId, enabled, merged, profile } = resolveZalouserAccountBase(params); return { accountId, diff --git a/extensions/zalouser/src/channel.test.ts b/extensions/zalouser/src/channel.test.ts index f54539ed809..321df502b38 100644 --- a/extensions/zalouser/src/channel.test.ts +++ b/extensions/zalouser/src/channel.test.ts @@ -15,6 +15,33 @@ vi.mock("./send.js", async (importOriginal) => { const mockSendMessage = vi.mocked(sendMessageZalouser); const mockSendReaction = vi.mocked(sendReactionZalouser); +function getResolveToolPolicy() { + const resolveToolPolicy = zalouserPlugin.groups?.resolveToolPolicy; + expect(resolveToolPolicy).toBeTypeOf("function"); + if (!resolveToolPolicy) { + throw new Error("resolveToolPolicy unavailable"); + } + return resolveToolPolicy; +} + +function resolveGroupToolPolicy( + groups: Record, + groupId: string, +) { + return getResolveToolPolicy()({ + cfg: { + channels: { + zalouser: { + groups, + }, + }, + }, + accountId: "default", + groupId, + groupChannel: groupId, + }); +} + describe("zalouser outbound", () => { beforeEach(() => { mockSendMessage.mockClear(); @@ -93,48 +120,12 @@ describe("zalouser channel policies", () => { }); it("resolves group tool policy by explicit group id", () => { - const resolveToolPolicy = zalouserPlugin.groups?.resolveToolPolicy; - expect(resolveToolPolicy).toBeTypeOf("function"); - if (!resolveToolPolicy) { - return; - } - const policy = resolveToolPolicy({ - cfg: { - channels: { - zalouser: { - groups: { - "123": { tools: { allow: ["search"] } }, - }, - }, - }, - }, - accountId: "default", - groupId: "123", - groupChannel: "123", - }); + const policy = resolveGroupToolPolicy({ "123": { tools: { allow: ["search"] } } }, "123"); expect(policy).toEqual({ allow: ["search"] }); }); it("falls back to wildcard group policy", () => { - const resolveToolPolicy = zalouserPlugin.groups?.resolveToolPolicy; - expect(resolveToolPolicy).toBeTypeOf("function"); - if (!resolveToolPolicy) { - return; - } - const policy = resolveToolPolicy({ - cfg: { - channels: { - zalouser: { - groups: { - "*": { tools: { deny: ["system.run"] } }, - }, - }, - }, - }, - accountId: "default", - groupId: "missing", - groupChannel: "missing", - }); + const policy = resolveGroupToolPolicy({ "*": { tools: { deny: ["system.run"] } } }, "missing"); expect(policy).toEqual({ deny: ["system.run"] }); }); diff --git a/src/acp/conversation-id.ts b/src/acp/conversation-id.ts index 7281fef4924..9cf17c9a579 100644 --- a/src/acp/conversation-id.ts +++ b/src/acp/conversation-id.ts @@ -4,7 +4,7 @@ export type ParsedTelegramTopicConversation = { canonicalConversationId: string; }; -function normalizeText(value: unknown): string { +export function normalizeConversationText(value: unknown): string { if (typeof value === "string") { return value.trim(); } @@ -15,7 +15,7 @@ function normalizeText(value: unknown): string { } export function parseTelegramChatIdFromTarget(raw: unknown): string | undefined { - const text = normalizeText(raw); + const text = normalizeConversationText(raw); if (!text) { return undefined; } diff --git a/src/acp/persistent-bindings.resolve.ts b/src/acp/persistent-bindings.resolve.ts index c69f1afe5af..84f052797ad 100644 --- a/src/acp/persistent-bindings.resolve.ts +++ b/src/acp/persistent-bindings.resolve.ts @@ -117,6 +117,70 @@ function toConfiguredBindingSpec(params: { }; } +function resolveConfiguredBindingRecord(params: { + cfg: OpenClawConfig; + bindings: AgentAcpBinding[]; + channel: ConfiguredAcpBindingChannel; + accountId: string; + selectConversation: ( + binding: AgentAcpBinding, + ) => { conversationId: string; parentConversationId?: string } | null; +}): ResolvedConfiguredAcpBinding | null { + let wildcardMatch: { + binding: AgentAcpBinding; + conversationId: string; + parentConversationId?: string; + } | null = null; + for (const binding of params.bindings) { + if (normalizeBindingChannel(binding.match.channel) !== params.channel) { + continue; + } + const accountMatchPriority = resolveAccountMatchPriority( + binding.match.accountId, + params.accountId, + ); + if (accountMatchPriority === 0) { + continue; + } + const conversation = params.selectConversation(binding); + if (!conversation) { + continue; + } + const spec = toConfiguredBindingSpec({ + cfg: params.cfg, + channel: params.channel, + accountId: params.accountId, + conversationId: conversation.conversationId, + parentConversationId: conversation.parentConversationId, + binding, + }); + if (accountMatchPriority === 2) { + return { + spec, + record: toConfiguredAcpBindingRecord(spec), + }; + } + if (!wildcardMatch) { + wildcardMatch = { binding, ...conversation }; + } + } + if (!wildcardMatch) { + return null; + } + const spec = toConfiguredBindingSpec({ + cfg: params.cfg, + channel: params.channel, + accountId: params.accountId, + conversationId: wildcardMatch.conversationId, + parentConversationId: wildcardMatch.parentConversationId, + binding: wildcardMatch.binding, + }); + return { + spec, + record: toConfiguredAcpBindingRecord(spec), + }; +} + export function resolveConfiguredAcpBindingSpecBySessionKey(params: { cfg: OpenClawConfig; sessionKey: string; @@ -207,57 +271,20 @@ export function resolveConfiguredAcpBindingRecord(params: { if (channel === "discord") { const bindings = listAcpBindings(params.cfg); - const resolveDiscordBindingForConversation = ( - targetConversationId: string, - ): ResolvedConfiguredAcpBinding | null => { - let wildcardMatch: AgentAcpBinding | null = null; - for (const binding of bindings) { - if (normalizeBindingChannel(binding.match.channel) !== "discord") { - continue; - } - const accountMatchPriority = resolveAccountMatchPriority( - binding.match.accountId, - accountId, - ); - if (accountMatchPriority === 0) { - continue; - } - const bindingConversationId = resolveBindingConversationId(binding); - if (!bindingConversationId || bindingConversationId !== targetConversationId) { - continue; - } - if (accountMatchPriority === 2) { - const spec = toConfiguredBindingSpec({ - cfg: params.cfg, - channel: "discord", - accountId, - conversationId: targetConversationId, - binding, - }); - return { - spec, - record: toConfiguredAcpBindingRecord(spec), - }; - } - if (!wildcardMatch) { - wildcardMatch = binding; - } - } - if (wildcardMatch) { - const spec = toConfiguredBindingSpec({ - cfg: params.cfg, - channel: "discord", - accountId, - conversationId: targetConversationId, - binding: wildcardMatch, - }); - return { - spec, - record: toConfiguredAcpBindingRecord(spec), - }; - } - return null; - }; + const resolveDiscordBindingForConversation = (targetConversationId: string) => + resolveConfiguredBindingRecord({ + cfg: params.cfg, + bindings, + channel: "discord", + accountId, + selectConversation: (binding) => { + const bindingConversationId = resolveBindingConversationId(binding); + if (!bindingConversationId || bindingConversationId !== targetConversationId) { + return null; + } + return { conversationId: targetConversationId }; + }, + }); const directMatch = resolveDiscordBindingForConversation(conversationId); if (directMatch) { @@ -280,61 +307,31 @@ export function resolveConfiguredAcpBindingRecord(params: { if (!parsed || !parsed.chatId.startsWith("-")) { return null; } - let wildcardMatch: AgentAcpBinding | null = null; - for (const binding of listAcpBindings(params.cfg)) { - if (normalizeBindingChannel(binding.match.channel) !== "telegram") { - continue; - } - const accountMatchPriority = resolveAccountMatchPriority(binding.match.accountId, accountId); - if (accountMatchPriority === 0) { - continue; - } - const targetConversationId = resolveBindingConversationId(binding); - if (!targetConversationId) { - continue; - } - const targetParsed = parseTelegramTopicConversation({ - conversationId: targetConversationId, - }); - if (!targetParsed || !targetParsed.chatId.startsWith("-")) { - continue; - } - if (targetParsed.canonicalConversationId !== parsed.canonicalConversationId) { - continue; - } - if (accountMatchPriority === 2) { - const spec = toConfiguredBindingSpec({ - cfg: params.cfg, - channel: "telegram", - accountId, + return resolveConfiguredBindingRecord({ + cfg: params.cfg, + bindings: listAcpBindings(params.cfg), + channel: "telegram", + accountId, + selectConversation: (binding) => { + const targetConversationId = resolveBindingConversationId(binding); + if (!targetConversationId) { + return null; + } + const targetParsed = parseTelegramTopicConversation({ + conversationId: targetConversationId, + }); + if (!targetParsed || !targetParsed.chatId.startsWith("-")) { + return null; + } + if (targetParsed.canonicalConversationId !== parsed.canonicalConversationId) { + return null; + } + return { conversationId: parsed.canonicalConversationId, parentConversationId: parsed.chatId, - binding, - }); - return { - spec, - record: toConfiguredAcpBindingRecord(spec), }; - } - if (!wildcardMatch) { - wildcardMatch = binding; - } - } - if (wildcardMatch) { - const spec = toConfiguredBindingSpec({ - cfg: params.cfg, - channel: "telegram", - accountId, - conversationId: parsed.canonicalConversationId, - parentConversationId: parsed.chatId, - binding: wildcardMatch, - }); - return { - spec, - record: toConfiguredAcpBindingRecord(spec), - }; - } - return null; + }, + }); } return null; diff --git a/src/acp/persistent-bindings.test.ts b/src/acp/persistent-bindings.test.ts index deafbc53e15..30e74c05082 100644 --- a/src/acp/persistent-bindings.test.ts +++ b/src/acp/persistent-bindings.test.ts @@ -30,6 +30,10 @@ import { resolveConfiguredAcpBindingSpecBySessionKey, } from "./persistent-bindings.js"; +type ConfiguredBinding = NonNullable[number]; +type BindingRecordInput = Parameters[0]; +type BindingSpec = Parameters[0]["spec"]; + const baseCfg = { session: { mainKey: "main", scope: "per-sender" }, agents: { @@ -37,6 +41,105 @@ const baseCfg = { }, } satisfies OpenClawConfig; +const defaultDiscordConversationId = "1478836151241412759"; +const defaultDiscordAccountId = "default"; + +function createCfgWithBindings( + bindings: ConfiguredBinding[], + overrides?: Partial, +): OpenClawConfig { + return { + ...baseCfg, + ...overrides, + bindings, + } as OpenClawConfig; +} + +function createDiscordBinding(params: { + agentId: string; + conversationId: string; + accountId?: string; + acp?: Record; +}): ConfiguredBinding { + return { + type: "acp", + agentId: params.agentId, + match: { + channel: "discord", + accountId: params.accountId ?? defaultDiscordAccountId, + peer: { kind: "channel", id: params.conversationId }, + }, + ...(params.acp ? { acp: params.acp } : {}), + } as ConfiguredBinding; +} + +function createTelegramGroupBinding(params: { + agentId: string; + conversationId: string; + acp?: Record; +}): ConfiguredBinding { + return { + type: "acp", + agentId: params.agentId, + match: { + channel: "telegram", + accountId: defaultDiscordAccountId, + peer: { kind: "group", id: params.conversationId }, + }, + ...(params.acp ? { acp: params.acp } : {}), + } as ConfiguredBinding; +} + +function resolveBindingRecord(cfg: OpenClawConfig, overrides: Partial = {}) { + return resolveConfiguredAcpBindingRecord({ + cfg, + channel: "discord", + accountId: defaultDiscordAccountId, + conversationId: defaultDiscordConversationId, + ...overrides, + }); +} + +function resolveDiscordBindingSpecBySession( + cfg: OpenClawConfig, + conversationId = defaultDiscordConversationId, +) { + const resolved = resolveBindingRecord(cfg, { conversationId }); + return resolveConfiguredAcpBindingSpecBySessionKey({ + cfg, + sessionKey: resolved?.record.targetSessionKey ?? "", + }); +} + +function createDiscordPersistentSpec(overrides: Partial = {}): BindingSpec { + return { + channel: "discord", + accountId: defaultDiscordAccountId, + conversationId: defaultDiscordConversationId, + agentId: "codex", + mode: "persistent", + ...overrides, + } as BindingSpec; +} + +function mockReadySession(params: { spec: BindingSpec; cwd: string }) { + const sessionKey = buildConfiguredAcpSessionKey(params.spec); + managerMocks.resolveSession.mockReturnValue({ + kind: "ready", + sessionKey, + meta: { + backend: "acpx", + agent: params.spec.acpAgentId ?? params.spec.agentId, + runtimeSessionName: "existing", + mode: params.spec.mode, + runtimeOptions: { cwd: params.cwd }, + state: "idle", + lastActivityAt: Date.now(), + }, + }); + return sessionKey; +} + beforeEach(() => { managerMocks.resolveSession.mockReset(); managerMocks.closeSession.mockReset().mockResolvedValue({ @@ -50,58 +153,30 @@ beforeEach(() => { describe("resolveConfiguredAcpBindingRecord", () => { it("resolves discord channel ACP binding from top-level typed bindings", () => { - const cfg = { - ...baseCfg, - bindings: [ - { - type: "acp", - agentId: "codex", - match: { - channel: "discord", - accountId: "default", - peer: { kind: "channel", id: "1478836151241412759" }, - }, - acp: { - cwd: "/repo/openclaw", - }, - }, - ], - } satisfies OpenClawConfig; - - const resolved = resolveConfiguredAcpBindingRecord({ - cfg, - channel: "discord", - accountId: "default", - conversationId: "1478836151241412759", - }); + const cfg = createCfgWithBindings([ + createDiscordBinding({ + agentId: "codex", + conversationId: defaultDiscordConversationId, + acp: { cwd: "/repo/openclaw" }, + }), + ]); + const resolved = resolveBindingRecord(cfg); expect(resolved?.spec.channel).toBe("discord"); - expect(resolved?.spec.conversationId).toBe("1478836151241412759"); + expect(resolved?.spec.conversationId).toBe(defaultDiscordConversationId); expect(resolved?.spec.agentId).toBe("codex"); expect(resolved?.record.targetSessionKey).toContain("agent:codex:acp:binding:discord:default:"); expect(resolved?.record.metadata?.source).toBe("config"); }); it("falls back to parent discord channel when conversation is a thread id", () => { - const cfg = { - ...baseCfg, - bindings: [ - { - type: "acp", - agentId: "codex", - match: { - channel: "discord", - accountId: "default", - peer: { kind: "channel", id: "channel-parent-1" }, - }, - }, - ], - } satisfies OpenClawConfig; - - const resolved = resolveConfiguredAcpBindingRecord({ - cfg, - channel: "discord", - accountId: "default", + const cfg = createCfgWithBindings([ + createDiscordBinding({ + agentId: "codex", + conversationId: "channel-parent-1", + }), + ]); + const resolved = resolveBindingRecord(cfg, { conversationId: "thread-123", parentConversationId: "channel-parent-1", }); @@ -111,34 +186,17 @@ describe("resolveConfiguredAcpBindingRecord", () => { }); it("prefers direct discord thread binding over parent channel fallback", () => { - const cfg = { - ...baseCfg, - bindings: [ - { - type: "acp", - agentId: "codex", - match: { - channel: "discord", - accountId: "default", - peer: { kind: "channel", id: "channel-parent-1" }, - }, - }, - { - type: "acp", - agentId: "claude", - match: { - channel: "discord", - accountId: "default", - peer: { kind: "channel", id: "thread-123" }, - }, - }, - ], - } satisfies OpenClawConfig; - - const resolved = resolveConfiguredAcpBindingRecord({ - cfg, - channel: "discord", - accountId: "default", + const cfg = createCfgWithBindings([ + createDiscordBinding({ + agentId: "codex", + conversationId: "channel-parent-1", + }), + createDiscordBinding({ + agentId: "claude", + conversationId: "thread-123", + }), + ]); + const resolved = resolveBindingRecord(cfg, { conversationId: "thread-123", parentConversationId: "channel-parent-1", }); @@ -148,60 +206,30 @@ describe("resolveConfiguredAcpBindingRecord", () => { }); it("prefers exact account binding over wildcard for the same discord conversation", () => { - const cfg = { - ...baseCfg, - bindings: [ - { - type: "acp", - agentId: "codex", - match: { - channel: "discord", - accountId: "*", - peer: { kind: "channel", id: "1478836151241412759" }, - }, - }, - { - type: "acp", - agentId: "claude", - match: { - channel: "discord", - accountId: "default", - peer: { kind: "channel", id: "1478836151241412759" }, - }, - }, - ], - } satisfies OpenClawConfig; - - const resolved = resolveConfiguredAcpBindingRecord({ - cfg, - channel: "discord", - accountId: "default", - conversationId: "1478836151241412759", - }); + const cfg = createCfgWithBindings([ + createDiscordBinding({ + agentId: "codex", + conversationId: defaultDiscordConversationId, + accountId: "*", + }), + createDiscordBinding({ + agentId: "claude", + conversationId: defaultDiscordConversationId, + }), + ]); + const resolved = resolveBindingRecord(cfg); expect(resolved?.spec.agentId).toBe("claude"); }); it("returns null when no top-level ACP binding matches the conversation", () => { - const cfg = { - ...baseCfg, - bindings: [ - { - type: "acp", - agentId: "codex", - match: { - channel: "discord", - accountId: "default", - peer: { kind: "channel", id: "different-channel" }, - }, - }, - ], - } satisfies OpenClawConfig; - - const resolved = resolveConfiguredAcpBindingRecord({ - cfg, - channel: "discord", - accountId: "default", + const cfg = createCfgWithBindings([ + createDiscordBinding({ + agentId: "codex", + conversationId: "different-channel", + }), + ]); + const resolved = resolveBindingRecord(cfg, { conversationId: "thread-123", parentConversationId: "channel-parent-1", }); @@ -210,23 +238,13 @@ describe("resolveConfiguredAcpBindingRecord", () => { }); it("resolves telegram forum topic bindings using canonical conversation ids", () => { - const cfg = { - ...baseCfg, - bindings: [ - { - type: "acp", - agentId: "claude", - match: { - channel: "telegram", - accountId: "default", - peer: { kind: "group", id: "-1001234567890:topic:42" }, - }, - acp: { - backend: "acpx", - }, - }, - ], - } satisfies OpenClawConfig; + const cfg = createCfgWithBindings([ + createTelegramGroupBinding({ + agentId: "claude", + conversationId: "-1001234567890:topic:42", + acp: { backend: "acpx" }, + }), + ]); const canonical = resolveConfiguredAcpBindingRecord({ cfg, @@ -250,20 +268,12 @@ describe("resolveConfiguredAcpBindingRecord", () => { }); it("skips telegram non-group topic configs", () => { - const cfg = { - ...baseCfg, - bindings: [ - { - type: "acp", - agentId: "claude", - match: { - channel: "telegram", - accountId: "default", - peer: { kind: "group", id: "123456789:topic:42" }, - }, - }, - ], - } satisfies OpenClawConfig; + const cfg = createCfgWithBindings([ + createTelegramGroupBinding({ + agentId: "claude", + conversationId: "123456789:topic:42", + }), + ]); const resolved = resolveConfiguredAcpBindingRecord({ cfg, @@ -275,44 +285,34 @@ describe("resolveConfiguredAcpBindingRecord", () => { }); it("applies agent runtime ACP defaults for bound conversations", () => { - const cfg = { - ...baseCfg, - agents: { - list: [ - { id: "main" }, - { - id: "coding", - runtime: { - type: "acp", - acp: { - agent: "codex", - backend: "acpx", - mode: "oneshot", - cwd: "/workspace/repo-a", + const cfg = createCfgWithBindings( + [ + createDiscordBinding({ + agentId: "coding", + conversationId: defaultDiscordConversationId, + }), + ], + { + agents: { + list: [ + { id: "main" }, + { + id: "coding", + runtime: { + type: "acp", + acp: { + agent: "codex", + backend: "acpx", + mode: "oneshot", + cwd: "/workspace/repo-a", + }, }, }, - }, - ], - }, - bindings: [ - { - type: "acp", - agentId: "coding", - match: { - channel: "discord", - accountId: "default", - peer: { kind: "channel", id: "1478836151241412759" }, - }, + ], }, - ], - } satisfies OpenClawConfig; - - const resolved = resolveConfiguredAcpBindingRecord({ - cfg, - channel: "discord", - accountId: "default", - conversationId: "1478836151241412759", - }); + }, + ); + const resolved = resolveBindingRecord(cfg); expect(resolved?.spec.agentId).toBe("coding"); expect(resolved?.spec.acpAgentId).toBe("codex"); @@ -324,37 +324,17 @@ describe("resolveConfiguredAcpBindingRecord", () => { describe("resolveConfiguredAcpBindingSpecBySessionKey", () => { it("maps a configured discord binding session key back to its spec", () => { - const cfg = { - ...baseCfg, - bindings: [ - { - type: "acp", - agentId: "codex", - match: { - channel: "discord", - accountId: "default", - peer: { kind: "channel", id: "1478836151241412759" }, - }, - acp: { - backend: "acpx", - }, - }, - ], - } satisfies OpenClawConfig; - - const resolved = resolveConfiguredAcpBindingRecord({ - cfg, - channel: "discord", - accountId: "default", - conversationId: "1478836151241412759", - }); - const spec = resolveConfiguredAcpBindingSpecBySessionKey({ - cfg, - sessionKey: resolved?.record.targetSessionKey ?? "", - }); + const cfg = createCfgWithBindings([ + createDiscordBinding({ + agentId: "codex", + conversationId: defaultDiscordConversationId, + acp: { backend: "acpx" }, + }), + ]); + const spec = resolveDiscordBindingSpecBySession(cfg); expect(spec?.channel).toBe("discord"); - expect(spec?.conversationId).toBe("1478836151241412759"); + expect(spec?.conversationId).toBe(defaultDiscordConversationId); expect(spec?.agentId).toBe("codex"); expect(spec?.backend).toBe("acpx"); }); @@ -368,46 +348,20 @@ describe("resolveConfiguredAcpBindingSpecBySessionKey", () => { }); it("prefers exact account ACP settings over wildcard when session keys collide", () => { - const cfg = { - ...baseCfg, - bindings: [ - { - type: "acp", - agentId: "codex", - match: { - channel: "discord", - accountId: "*", - peer: { kind: "channel", id: "1478836151241412759" }, - }, - acp: { - backend: "wild", - }, - }, - { - type: "acp", - agentId: "codex", - match: { - channel: "discord", - accountId: "default", - peer: { kind: "channel", id: "1478836151241412759" }, - }, - acp: { - backend: "exact", - }, - }, - ], - } satisfies OpenClawConfig; - - const resolved = resolveConfiguredAcpBindingRecord({ - cfg, - channel: "discord", - accountId: "default", - conversationId: "1478836151241412759", - }); - const spec = resolveConfiguredAcpBindingSpecBySessionKey({ - cfg, - sessionKey: resolved?.record.targetSessionKey ?? "", - }); + const cfg = createCfgWithBindings([ + createDiscordBinding({ + agentId: "codex", + conversationId: defaultDiscordConversationId, + accountId: "*", + acp: { backend: "wild" }, + }), + createDiscordBinding({ + agentId: "codex", + conversationId: defaultDiscordConversationId, + acp: { backend: "exact" }, + }), + ]); + const spec = resolveDiscordBindingSpecBySession(cfg); expect(spec?.backend).toBe("exact"); }); @@ -435,26 +389,10 @@ describe("buildConfiguredAcpSessionKey", () => { describe("ensureConfiguredAcpBindingSession", () => { it("keeps an existing ready session when configured binding omits cwd", async () => { - const spec = { - channel: "discord" as const, - accountId: "default", - conversationId: "1478836151241412759", - agentId: "codex", - mode: "persistent" as const, - }; - const sessionKey = buildConfiguredAcpSessionKey(spec); - managerMocks.resolveSession.mockReturnValue({ - kind: "ready", - sessionKey, - meta: { - backend: "acpx", - agent: "codex", - runtimeSessionName: "existing", - mode: "persistent", - runtimeOptions: { cwd: "/workspace/openclaw" }, - state: "idle", - lastActivityAt: Date.now(), - }, + const spec = createDiscordPersistentSpec(); + const sessionKey = mockReadySession({ + spec, + cwd: "/workspace/openclaw", }); const ensured = await ensureConfiguredAcpBindingSession({ @@ -468,27 +406,12 @@ describe("ensureConfiguredAcpBindingSession", () => { }); it("reinitializes a ready session when binding config explicitly sets mismatched cwd", async () => { - const spec = { - channel: "discord" as const, - accountId: "default", - conversationId: "1478836151241412759", - agentId: "codex", - mode: "persistent" as const, + const spec = createDiscordPersistentSpec({ cwd: "/workspace/repo-a", - }; - const sessionKey = buildConfiguredAcpSessionKey(spec); - managerMocks.resolveSession.mockReturnValue({ - kind: "ready", - sessionKey, - meta: { - backend: "acpx", - agent: "codex", - runtimeSessionName: "existing", - mode: "persistent", - runtimeOptions: { cwd: "/workspace/other-repo" }, - state: "idle", - lastActivityAt: Date.now(), - }, + }); + const sessionKey = mockReadySession({ + spec, + cwd: "/workspace/other-repo", }); const ensured = await ensureConfiguredAcpBindingSession({ @@ -508,14 +431,10 @@ describe("ensureConfiguredAcpBindingSession", () => { }); it("initializes ACP session with runtime agent override when provided", async () => { - const spec = { - channel: "discord" as const, - accountId: "default", - conversationId: "1478836151241412759", + const spec = createDiscordPersistentSpec({ agentId: "coding", acpAgentId: "codex", - mode: "persistent" as const, - }; + }); managerMocks.resolveSession.mockReturnValue({ kind: "none" }); const ensured = await ensureConfiguredAcpBindingSession({ @@ -534,24 +453,16 @@ describe("ensureConfiguredAcpBindingSession", () => { describe("resetAcpSessionInPlace", () => { it("reinitializes from configured binding when ACP metadata is missing", async () => { - const cfg = { - ...baseCfg, - bindings: [ - { - type: "acp", - agentId: "claude", - match: { - channel: "discord", - accountId: "default", - peer: { kind: "channel", id: "1478844424791396446" }, - }, - acp: { - mode: "persistent", - backend: "acpx", - }, + const cfg = createCfgWithBindings([ + createDiscordBinding({ + agentId: "claude", + conversationId: "1478844424791396446", + acp: { + mode: "persistent", + backend: "acpx", }, - ], - } satisfies OpenClawConfig; + }), + ]); const sessionKey = buildConfiguredAcpSessionKey({ channel: "discord", accountId: "default", diff --git a/src/acp/server.startup.test.ts b/src/acp/server.startup.test.ts index 2f9b96d8511..35c43478ec9 100644 --- a/src/acp/server.startup.test.ts +++ b/src/acp/server.startup.test.ts @@ -129,6 +129,22 @@ describe("serveAcpGateway startup", () => { return { signalHandlers, onceSpy }; } + async function emitHelloAndWaitForAgentSideConnection() { + const gateway = getMockGateway(); + gateway.emitHello(); + await vi.waitFor(() => { + expect(mockState.agentSideConnectionCtor).toHaveBeenCalledTimes(1); + }); + } + + async function stopServeWithSigint( + signalHandlers: Map void>, + servePromise: Promise, + ) { + signalHandlers.get("SIGINT")?.(); + await servePromise; + } + beforeAll(async () => { ({ serveAcpGateway } = await import("./server.js")); }); @@ -153,14 +169,8 @@ describe("serveAcpGateway startup", () => { await Promise.resolve(); expect(mockState.agentSideConnectionCtor).not.toHaveBeenCalled(); - const gateway = getMockGateway(); - gateway.emitHello(); - await vi.waitFor(() => { - expect(mockState.agentSideConnectionCtor).toHaveBeenCalledTimes(1); - }); - - signalHandlers.get("SIGINT")?.(); - await servePromise; + await emitHelloAndWaitForAgentSideConnection(); + await stopServeWithSigint(signalHandlers, servePromise); } finally { onceSpy.mockRestore(); } @@ -207,13 +217,8 @@ describe("serveAcpGateway startup", () => { password: "resolved-secret-password", // pragma: allowlist secret }); - const gateway = getMockGateway(); - gateway.emitHello(); - await vi.waitFor(() => { - expect(mockState.agentSideConnectionCtor).toHaveBeenCalledTimes(1); - }); - signalHandlers.get("SIGINT")?.(); - await servePromise; + await emitHelloAndWaitForAgentSideConnection(); + await stopServeWithSigint(signalHandlers, servePromise); } finally { onceSpy.mockRestore(); } @@ -236,13 +241,8 @@ describe("serveAcpGateway startup", () => { }), ); - const gateway = getMockGateway(); - gateway.emitHello(); - await vi.waitFor(() => { - expect(mockState.agentSideConnectionCtor).toHaveBeenCalledTimes(1); - }); - signalHandlers.get("SIGINT")?.(); - await servePromise; + await emitHelloAndWaitForAgentSideConnection(); + await stopServeWithSigint(signalHandlers, servePromise); } finally { onceSpy.mockRestore(); } diff --git a/src/acp/translator.cancel-scoping.test.ts b/src/acp/translator.cancel-scoping.test.ts index c84832369a0..e862222f7a0 100644 --- a/src/acp/translator.cancel-scoping.test.ts +++ b/src/acp/translator.cancel-scoping.test.ts @@ -91,19 +91,45 @@ async function startPendingPrompt( }; } +async function cancelAndExpectAbortForPendingRun( + harness: Harness, + sessionId: string, + sessionKey: string, + pending: { promptPromise: Promise; runId: string }, +) { + await harness.agent.cancel({ sessionId } as CancelNotification); + + expect(harness.requestSpy).toHaveBeenCalledWith("chat.abort", { + sessionKey, + runId: pending.runId, + }); + await expect(pending.promptPromise).resolves.toEqual({ stopReason: "cancelled" }); +} + +async function deliverFinalChatEventAndExpectEndTurn( + harness: Harness, + sessionKey: string, + pending: { promptPromise: Promise; runId: string }, + seq: number, +) { + await harness.agent.handleGatewayEvent( + createChatEvent({ + runId: pending.runId, + sessionKey, + seq, + state: "final", + }), + ); + await expect(pending.promptPromise).resolves.toEqual({ stopReason: "end_turn" }); +} + describe("acp translator cancel and run scoping", () => { it("cancel passes active runId to chat.abort", async () => { const sessionKey = "agent:main:shared"; const harness = createHarness([{ sessionId: "session-1", sessionKey }]); const pending = await startPendingPrompt(harness, "session-1"); - await harness.agent.cancel({ sessionId: "session-1" } as CancelNotification); - - expect(harness.requestSpy).toHaveBeenCalledWith("chat.abort", { - sessionKey, - runId: pending.runId, - }); - await expect(pending.promptPromise).resolves.toEqual({ stopReason: "cancelled" }); + await cancelAndExpectAbortForPendingRun(harness, "session-1", sessionKey, pending); }); it("cancel uses pending runId when there is no active run", async () => { @@ -112,13 +138,7 @@ describe("acp translator cancel and run scoping", () => { const pending = await startPendingPrompt(harness, "session-1"); harness.sessionStore.clearActiveRun("session-1"); - await harness.agent.cancel({ sessionId: "session-1" } as CancelNotification); - - expect(harness.requestSpy).toHaveBeenCalledWith("chat.abort", { - sessionKey, - runId: pending.runId, - }); - await expect(pending.promptPromise).resolves.toEqual({ stopReason: "cancelled" }); + await cancelAndExpectAbortForPendingRun(harness, "session-1", sessionKey, pending); }); it("cancel skips chat.abort when there is no active run and no pending prompt", async () => { @@ -145,15 +165,7 @@ describe("acp translator cancel and run scoping", () => { expect(abortCalls).toHaveLength(0); expect(harness.sessionStore.getSession("session-2")?.activeRunId).toBe(pending2.runId); - await harness.agent.handleGatewayEvent( - createChatEvent({ - runId: pending2.runId, - sessionKey, - seq: 1, - state: "final", - }), - ); - await expect(pending2.promptPromise).resolves.toEqual({ stopReason: "end_turn" }); + await deliverFinalChatEventAndExpectEndTurn(harness, sessionKey, pending2, 1); }); it("drops chat events when runId does not match the active prompt", async () => { @@ -250,15 +262,7 @@ describe("acp translator cancel and run scoping", () => { ); expect(harness.sessionUpdateSpy).toHaveBeenCalledTimes(1); - await harness.agent.handleGatewayEvent( - createChatEvent({ - runId: pending2.runId, - sessionKey, - seq: 1, - state: "final", - }), - ); - await expect(pending2.promptPromise).resolves.toEqual({ stopReason: "end_turn" }); + await deliverFinalChatEventAndExpectEndTurn(harness, sessionKey, pending2, 1); expect(harness.sessionStore.getSession("session-1")?.activeRunId).toBe(pending1.runId); await harness.agent.handleGatewayEvent( diff --git a/src/agents/openclaw-tools.ts b/src/agents/openclaw-tools.ts index ea12b5121d8..25b5cae0f59 100644 --- a/src/agents/openclaw-tools.ts +++ b/src/agents/openclaw-tools.ts @@ -87,15 +87,16 @@ export function createOpenClawTools( options?.spawnWorkspaceDir ?? options?.workspaceDir, ); const runtimeWebTools = getActiveRuntimeWebToolsMetadata(); + const sandbox = + options?.sandboxRoot && options?.sandboxFsBridge + ? { root: options.sandboxRoot, bridge: options.sandboxFsBridge } + : undefined; const imageTool = options?.agentDir?.trim() ? createImageTool({ config: options?.config, agentDir: options.agentDir, workspaceDir, - sandbox: - options?.sandboxRoot && options?.sandboxFsBridge - ? { root: options.sandboxRoot, bridge: options.sandboxFsBridge } - : undefined, + sandbox, fsPolicy: options?.fsPolicy, modelHasVision: options?.modelHasVision, }) @@ -105,10 +106,7 @@ export function createOpenClawTools( config: options?.config, agentDir: options.agentDir, workspaceDir, - sandbox: - options?.sandboxRoot && options?.sandboxFsBridge - ? { root: options.sandboxRoot, bridge: options.sandboxFsBridge } - : undefined, + sandbox, fsPolicy: options?.fsPolicy, }) : null; diff --git a/src/agents/tools/common.ts b/src/agents/tools/common.ts index 19cca2d7927..81d3f4efc00 100644 --- a/src/agents/tools/common.ts +++ b/src/agents/tools/common.ts @@ -1,6 +1,7 @@ import fs from "node:fs/promises"; import type { AgentTool, AgentToolResult } from "@mariozechner/pi-agent-core"; import { detectMime } from "../../media/mime.js"; +import { readSnakeCaseParamRaw } from "../../param-key.js"; import type { ImageSanitizationLimits } from "../image-sanitization.js"; import { sanitizeToolResultImages } from "../tool-images.js"; @@ -53,22 +54,8 @@ export function createActionGate>( }; } -function toSnakeCaseKey(key: string): string { - return key - .replace(/([A-Z]+)([A-Z][a-z])/g, "$1_$2") - .replace(/([a-z0-9])([A-Z])/g, "$1_$2") - .toLowerCase(); -} - function readParamRaw(params: Record, key: string): unknown { - if (Object.hasOwn(params, key)) { - return params[key]; - } - const snakeKey = toSnakeCaseKey(key); - if (snakeKey !== key && Object.hasOwn(params, snakeKey)) { - return params[snakeKey]; - } - return undefined; + return readSnakeCaseParamRaw(params, key); } export function readStringParam( diff --git a/src/auto-reply/reply/commands-acp/context.ts b/src/auto-reply/reply/commands-acp/context.ts index 16291713fda..fd90175f38a 100644 --- a/src/auto-reply/reply/commands-acp/context.ts +++ b/src/auto-reply/reply/commands-acp/context.ts @@ -1,5 +1,6 @@ import { buildTelegramTopicConversationId, + normalizeConversationText, parseTelegramChatIdFromTarget, } from "../../../acp/conversation-id.js"; import { DISCORD_THREAD_BINDING_CHANNEL } from "../../../channels/thread-bindings-policy.js"; @@ -8,33 +9,25 @@ import { parseAgentSessionKey } from "../../../routing/session-key.js"; import type { HandleCommandsParams } from "../commands-types.js"; import { resolveTelegramConversationId } from "../telegram-context.js"; -function normalizeString(value: unknown): string { - if (typeof value === "string") { - return value.trim(); - } - if (typeof value === "number" || typeof value === "bigint" || typeof value === "boolean") { - return `${value}`.trim(); - } - return ""; -} - export function resolveAcpCommandChannel(params: HandleCommandsParams): string { const raw = params.ctx.OriginatingChannel ?? params.command.channel ?? params.ctx.Surface ?? params.ctx.Provider; - return normalizeString(raw).toLowerCase(); + return normalizeConversationText(raw).toLowerCase(); } export function resolveAcpCommandAccountId(params: HandleCommandsParams): string { - const accountId = normalizeString(params.ctx.AccountId); + const accountId = normalizeConversationText(params.ctx.AccountId); return accountId || "default"; } export function resolveAcpCommandThreadId(params: HandleCommandsParams): string | undefined { const threadId = - params.ctx.MessageThreadId != null ? normalizeString(String(params.ctx.MessageThreadId)) : ""; + params.ctx.MessageThreadId != null + ? normalizeConversationText(String(params.ctx.MessageThreadId)) + : ""; return threadId || undefined; } @@ -72,7 +65,7 @@ export function resolveAcpCommandConversationId(params: HandleCommandsParams): s } function parseDiscordParentChannelFromSessionKey(raw: unknown): string | undefined { - const sessionKey = normalizeString(raw); + const sessionKey = normalizeConversationText(raw); if (!sessionKey) { return undefined; } @@ -85,7 +78,7 @@ function parseDiscordParentChannelFromSessionKey(raw: unknown): string | undefin } function parseDiscordParentChannelFromContext(raw: unknown): string | undefined { - const parentId = normalizeString(raw); + const parentId = normalizeConversationText(raw); if (!parentId) { return undefined; } diff --git a/src/auto-reply/reply/session.ts b/src/auto-reply/reply/session.ts index 6db6b1708cb..85e6754025f 100644 --- a/src/auto-reply/reply/session.ts +++ b/src/auto-reply/reply/session.ts @@ -2,6 +2,7 @@ import crypto from "node:crypto"; import path from "node:path"; import { buildTelegramTopicConversationId, + normalizeConversationText, parseTelegramChatIdFromTarget, } from "../../acp/conversation-id.js"; import { resolveSessionAgentId } from "../../agents/agent-scope.js"; @@ -69,18 +70,8 @@ export type SessionInitResult = { triggerBodyNormalized: string; }; -function normalizeSessionText(value: unknown): string { - if (typeof value === "string") { - return value.trim(); - } - if (typeof value === "number" || typeof value === "bigint" || typeof value === "boolean") { - return `${value}`.trim(); - } - return ""; -} - function parseDiscordParentChannelFromSessionKey(raw: unknown): string | undefined { - const sessionKey = normalizeSessionText(raw); + const sessionKey = normalizeConversationText(raw); if (!sessionKey) { return undefined; } @@ -98,15 +89,15 @@ function resolveAcpResetBindingContext(ctx: MsgContext): { conversationId: string; parentConversationId?: string; } | null { - const channelRaw = normalizeSessionText( + const channelRaw = normalizeConversationText( ctx.OriginatingChannel ?? ctx.Surface ?? ctx.Provider ?? "", ).toLowerCase(); if (!channelRaw) { return null; } - const accountId = normalizeSessionText(ctx.AccountId) || "default"; + const accountId = normalizeConversationText(ctx.AccountId) || "default"; const normalizedThreadId = - ctx.MessageThreadId != null ? normalizeSessionText(String(ctx.MessageThreadId)) : ""; + ctx.MessageThreadId != null ? normalizeConversationText(String(ctx.MessageThreadId)) : ""; if (channelRaw === "telegram") { const parentConversationId = @@ -143,7 +134,7 @@ function resolveAcpResetBindingContext(ctx: MsgContext): { } let parentConversationId: string | undefined; if (channelRaw === "discord" && normalizedThreadId) { - const fromContext = normalizeSessionText(ctx.ThreadParentId); + const fromContext = normalizeConversationText(ctx.ThreadParentId); if (fromContext && fromContext !== conversationId) { parentConversationId = fromContext; } else { @@ -172,7 +163,7 @@ function resolveBoundAcpSessionForReset(params: { cfg: OpenClawConfig; ctx: MsgContext; }): string | undefined { - const activeSessionKey = normalizeSessionText(params.ctx.SessionKey); + const activeSessionKey = normalizeConversationText(params.ctx.SessionKey); const bindingContext = resolveAcpResetBindingContext(params.ctx); return resolveEffectiveResetTargetSessionKey({ cfg: params.cfg, diff --git a/src/config/paths.ts b/src/config/paths.ts index 5f9afc85a46..84c27749bcf 100644 --- a/src/config/paths.ts +++ b/src/config/paths.ts @@ -1,7 +1,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { expandHomePrefix, resolveRequiredHomeDir } from "../infra/home-dir.js"; +import { resolveHomeRelativePath, resolveRequiredHomeDir } from "../infra/home-dir.js"; import type { OpenClawConfig } from "./types.js"; /** @@ -93,19 +93,7 @@ function resolveUserPath( env: NodeJS.ProcessEnv = process.env, homedir: () => string = envHomedir(env), ): string { - const trimmed = input.trim(); - if (!trimmed) { - return trimmed; - } - if (trimmed.startsWith("~")) { - const expanded = expandHomePrefix(trimmed, { - home: resolveRequiredHomeDir(env, homedir), - env, - homedir, - }); - return path.resolve(expanded); - } - return path.resolve(trimmed); + return resolveHomeRelativePath(input, { env, homedir }); } export const STATE_DIR = resolveStateDir(); diff --git a/src/config/sessions/sessions.test.ts b/src/config/sessions/sessions.test.ts index 6866d6c10c1..2773b6d0fe7 100644 --- a/src/config/sessions/sessions.test.ts +++ b/src/config/sessions/sessions.test.ts @@ -283,18 +283,25 @@ describe("session store lock (Promise chain mutex)", () => { describe("appendAssistantMessageToSessionTranscript", () => { const fixture = useTempSessionsFixture("transcript-test-"); + const sessionId = "test-session-id"; + const sessionKey = "test-session"; + + function writeTranscriptStore() { + fs.writeFileSync( + fixture.storePath(), + JSON.stringify({ + [sessionKey]: { + sessionId, + chatType: "direct", + channel: "discord", + }, + }), + "utf-8", + ); + } it("creates transcript file and appends message for valid session", async () => { - const sessionId = "test-session-id"; - const sessionKey = "test-session"; - const store = { - [sessionKey]: { - sessionId, - chatType: "direct", - channel: "discord", - }, - }; - fs.writeFileSync(fixture.storePath(), JSON.stringify(store), "utf-8"); + writeTranscriptStore(); const result = await appendAssistantMessageToSessionTranscript({ sessionKey, @@ -326,16 +333,7 @@ describe("appendAssistantMessageToSessionTranscript", () => { }); it("does not append a duplicate delivery mirror for the same idempotency key", async () => { - const sessionId = "test-session-id"; - const sessionKey = "test-session"; - const store = { - [sessionKey]: { - sessionId, - chatType: "direct", - channel: "discord", - }, - }; - fs.writeFileSync(fixture.storePath(), JSON.stringify(store), "utf-8"); + writeTranscriptStore(); await appendAssistantMessageToSessionTranscript({ sessionKey, @@ -360,16 +358,7 @@ describe("appendAssistantMessageToSessionTranscript", () => { }); it("ignores malformed transcript lines when checking mirror idempotency", async () => { - const sessionId = "test-session-id"; - const sessionKey = "test-session"; - const store = { - [sessionKey]: { - sessionId, - chatType: "direct", - channel: "discord", - }, - }; - fs.writeFileSync(fixture.storePath(), JSON.stringify(store), "utf-8"); + writeTranscriptStore(); const sessionFile = resolveSessionTranscriptPathInDir(sessionId, fixture.sessionsDir()); fs.writeFileSync( diff --git a/src/cron/isolated-agent.model-formatting.test.ts b/src/cron/isolated-agent.model-formatting.test.ts index f9732a32d31..b09a9db5ea1 100644 --- a/src/cron/isolated-agent.model-formatting.test.ts +++ b/src/cron/isolated-agent.model-formatting.test.ts @@ -24,6 +24,8 @@ function lastEmbeddedCall(): { provider?: string; model?: string } { } const DEFAULT_MESSAGE = "do it"; +const DEFAULT_PROVIDER = "anthropic"; +const DEFAULT_MODEL = "claude-opus-4-5"; type TurnOptions = { cfgOverrides?: Parameters[2]; @@ -73,6 +75,50 @@ async function runTurn(home: string, options: TurnOptions = {}) { return { res, call: lastEmbeddedCall() }; } +function expectSelectedModel( + call: { provider?: string; model?: string }, + params: { provider: string; model: string }, +) { + expect(call.provider).toBe(params.provider); + expect(call.model).toBe(params.model); +} + +function expectDefaultSelectedModel(call: { provider?: string; model?: string }) { + expectSelectedModel(call, { provider: DEFAULT_PROVIDER, model: DEFAULT_MODEL }); +} + +function createCronSessionOverrideStore( + overrides: Record, + sessionId = "existing-session", +) { + return { + "agent:main:cron:job-1": { + sessionId, + updatedAt: Date.now(), + ...overrides, + }, + }; +} + +async function expectTurnModel( + home: string, + options: TurnOptions, + expected: { provider: string; model: string }, +) { + const { res, call } = await runTurn(home, options); + expect(res.status).toBe("ok"); + expectSelectedModel(call, expected); +} + +async function expectInvalidModel(home: string, model: string) { + const { res } = await runErrorTurn(home, { + jobPayload: { kind: "agentTurn", message: DEFAULT_MESSAGE, model }, + }); + expect(res.status).toBe("error"); + expect(res.error).toMatch(/invalid model/i); + expect(vi.mocked(runEmbeddedPiAgent)).not.toHaveBeenCalled(); +} + // --------------------------------------------------------------------------- // Tests // --------------------------------------------------------------------------- @@ -99,16 +145,17 @@ describe("cron model formatting and precedence edge cases", () => { it("handles leading/trailing whitespace in model string", async () => { await withTempHome(async (home) => { - const { res, call } = await runTurn(home, { - jobPayload: { - kind: "agentTurn", - message: DEFAULT_MESSAGE, - model: " openai/gpt-4.1-mini ", + await expectTurnModel( + home, + { + jobPayload: { + kind: "agentTurn", + message: DEFAULT_MESSAGE, + model: " openai/gpt-4.1-mini ", + }, }, - }); - expect(res.status).toBe("ok"); - expect(call.provider).toBe("openai"); - expect(call.model).toBe("gpt-4.1-mini"); + { provider: "openai", model: "gpt-4.1-mini" }, + ); }); }); @@ -129,38 +176,29 @@ describe("cron model formatting and precedence edge cases", () => { it("rejects model with trailing slash (empty model name)", async () => { await withTempHome(async (home) => { - const { res } = await runErrorTurn(home, { - jobPayload: { kind: "agentTurn", message: DEFAULT_MESSAGE, model: "openai/" }, - }); - expect(res.status).toBe("error"); - expect(res.error).toMatch(/invalid model/i); - expect(vi.mocked(runEmbeddedPiAgent)).not.toHaveBeenCalled(); + await expectInvalidModel(home, "openai/"); }); }); it("rejects model with leading slash (empty provider)", async () => { await withTempHome(async (home) => { - const { res } = await runErrorTurn(home, { - jobPayload: { kind: "agentTurn", message: DEFAULT_MESSAGE, model: "/gpt-4.1-mini" }, - }); - expect(res.status).toBe("error"); - expect(res.error).toMatch(/invalid model/i); - expect(vi.mocked(runEmbeddedPiAgent)).not.toHaveBeenCalled(); + await expectInvalidModel(home, "/gpt-4.1-mini"); }); }); it("normalizes provider casing", async () => { await withTempHome(async (home) => { - const { res, call } = await runTurn(home, { - jobPayload: { - kind: "agentTurn", - message: DEFAULT_MESSAGE, - model: "OpenAI/gpt-4.1-mini", + await expectTurnModel( + home, + { + jobPayload: { + kind: "agentTurn", + message: DEFAULT_MESSAGE, + model: "OpenAI/gpt-4.1-mini", + }, }, - }); - expect(res.status).toBe("ok"); - expect(call.provider).toBe("openai"); - expect(call.model).toBe("gpt-4.1-mini"); + { provider: "openai", model: "gpt-4.1-mini" }, + ); }); }); @@ -217,43 +255,39 @@ describe("cron model formatting and precedence edge cases", () => { // No model in job payload. Session store has openai override. // Provider must be openai, not the default anthropic. await withTempHome(async (home) => { - const { call } = await runTurn(home, { - jobPayload: { kind: "agentTurn", message: DEFAULT_MESSAGE, deliver: false }, - storeEntries: { - "agent:main:cron:job-1": { - sessionId: "existing-session", - updatedAt: Date.now(), + await expectTurnModel( + home, + { + jobPayload: { kind: "agentTurn", message: DEFAULT_MESSAGE, deliver: false }, + storeEntries: createCronSessionOverrideStore({ providerOverride: "openai", modelOverride: "gpt-4.1-mini", - }, + }), }, - }); - expect(call.provider).toBe("openai"); - expect(call.model).toBe("gpt-4.1-mini"); + { provider: "openai", model: "gpt-4.1-mini" }, + ); }); }); it("job payload model wins over conflicting session override", async () => { // Job payload says anthropic. Session says openai. Job must win. await withTempHome(async (home) => { - const { call } = await runTurn(home, { - jobPayload: { - kind: "agentTurn", - message: DEFAULT_MESSAGE, - model: "anthropic/claude-sonnet-4-5", - deliver: false, - }, - storeEntries: { - "agent:main:cron:job-1": { - sessionId: "existing-session", - updatedAt: Date.now(), + await expectTurnModel( + home, + { + jobPayload: { + kind: "agentTurn", + message: DEFAULT_MESSAGE, + model: "anthropic/claude-sonnet-4-5", + deliver: false, + }, + storeEntries: createCronSessionOverrideStore({ providerOverride: "openai", modelOverride: "gpt-4.1-mini", - }, + }), }, - }); - expect(call.provider).toBe("anthropic"); - expect(call.model).toBe("claude-sonnet-4-5"); + { provider: "anthropic", model: "claude-sonnet-4-5" }, + ); }); }); @@ -262,9 +296,7 @@ describe("cron model formatting and precedence edge cases", () => { const { call } = await runTurn(home, { jobPayload: { kind: "agentTurn", message: DEFAULT_MESSAGE, deliver: false }, }); - // makeCfg default is anthropic/claude-opus-4-5 - expect(call.provider).toBe("anthropic"); - expect(call.model).toBe("claude-opus-4-5"); + expectDefaultSelectedModel(call); }); }); }); @@ -293,17 +325,12 @@ describe("cron model formatting and precedence edge cases", () => { mockAgentPayloads([{ text: "ok" }]); const step2 = await runTurn(home, { jobPayload: { kind: "agentTurn", message: DEFAULT_MESSAGE, deliver: false }, - storeEntries: { - "agent:main:cron:job-1": { - sessionId: "existing-session", - updatedAt: Date.now(), - providerOverride: "openai", - modelOverride: "gpt-4.1-mini", - }, - }, + storeEntries: createCronSessionOverrideStore({ + providerOverride: "openai", + modelOverride: "gpt-4.1-mini", + }), }); - expect(step2.call.provider).toBe("openai"); - expect(step2.call.model).toBe("gpt-4.1-mini"); + expectSelectedModel(step2.call, { provider: "openai", model: "gpt-4.1-mini" }); // Step 3: Job payload says anthropic, session store still says openai vi.mocked(runEmbeddedPiAgent).mockClear(); @@ -315,17 +342,12 @@ describe("cron model formatting and precedence edge cases", () => { model: "anthropic/claude-opus-4-5", deliver: false, }, - storeEntries: { - "agent:main:cron:job-1": { - sessionId: "existing-session", - updatedAt: Date.now(), - providerOverride: "openai", - modelOverride: "gpt-4.1-mini", - }, - }, + storeEntries: createCronSessionOverrideStore({ + providerOverride: "openai", + modelOverride: "gpt-4.1-mini", + }), }); - expect(step3.call.provider).toBe("anthropic"); - expect(step3.call.model).toBe("claude-opus-4-5"); + expectSelectedModel(step3.call, { provider: "anthropic", model: "claude-opus-4-5" }); }); }); @@ -349,8 +371,7 @@ describe("cron model formatting and precedence edge cases", () => { const r2 = await runTurn(home, { jobPayload: { kind: "agentTurn", message: DEFAULT_MESSAGE, deliver: false }, }); - expect(r2.call.provider).toBe("anthropic"); - expect(r2.call.model).toBe("claude-opus-4-5"); + expectDefaultSelectedModel(r2.call); }); }); }); @@ -363,19 +384,20 @@ describe("cron model formatting and precedence edge cases", () => { // The stored modelOverride/providerOverride must still be read and applied // (resolveCronSession spreads ...entry before overriding core fields). await withTempHome(async (home) => { - const { call } = await runTurn(home, { - jobPayload: { kind: "agentTurn", message: DEFAULT_MESSAGE, deliver: false }, - storeEntries: { - "agent:main:cron:job-1": { - sessionId: "old-session-id", - updatedAt: Date.now(), - providerOverride: "openai", - modelOverride: "gpt-4.1-mini", - }, + await expectTurnModel( + home, + { + jobPayload: { kind: "agentTurn", message: DEFAULT_MESSAGE, deliver: false }, + storeEntries: createCronSessionOverrideStore( + { + providerOverride: "openai", + modelOverride: "gpt-4.1-mini", + }, + "old-session-id", + ), }, - }); - expect(call.provider).toBe("openai"); - expect(call.model).toBe("gpt-4.1-mini"); + { provider: "openai", model: "gpt-4.1-mini" }, + ); }); }); @@ -383,16 +405,9 @@ describe("cron model formatting and precedence edge cases", () => { await withTempHome(async (home) => { const { call } = await runTurn(home, { jobPayload: { kind: "agentTurn", message: DEFAULT_MESSAGE, deliver: false }, - storeEntries: { - "agent:main:cron:job-1": { - sessionId: "old-session-id", - updatedAt: Date.now(), - // No providerOverride or modelOverride - }, - }, + storeEntries: createCronSessionOverrideStore({}, "old-session-id"), }); - expect(call.provider).toBe("anthropic"); - expect(call.model).toBe("claude-opus-4-5"); + expectDefaultSelectedModel(call); }); }); }); @@ -405,8 +420,7 @@ describe("cron model formatting and precedence edge cases", () => { const { call } = await runTurn(home, { jobPayload: { kind: "agentTurn", message: DEFAULT_MESSAGE, model: " " }, }); - expect(call.provider).toBe("anthropic"); - expect(call.model).toBe("claude-opus-4-5"); + expectDefaultSelectedModel(call); }); }); @@ -415,8 +429,7 @@ describe("cron model formatting and precedence edge cases", () => { const { call } = await runTurn(home, { jobPayload: { kind: "agentTurn", message: DEFAULT_MESSAGE, model: "" }, }); - expect(call.provider).toBe("anthropic"); - expect(call.model).toBe("claude-opus-4-5"); + expectDefaultSelectedModel(call); }); }); @@ -424,18 +437,13 @@ describe("cron model formatting and precedence edge cases", () => { await withTempHome(async (home) => { const { call } = await runTurn(home, { jobPayload: { kind: "agentTurn", message: DEFAULT_MESSAGE, deliver: false }, - storeEntries: { - "agent:main:cron:job-1": { - sessionId: "old", - updatedAt: Date.now(), - providerOverride: "openai", - modelOverride: " ", - }, - }, + storeEntries: createCronSessionOverrideStore( + { providerOverride: "openai", modelOverride: " " }, + "old", + ), }); // Whitespace modelOverride should be ignored → default - expect(call.provider).toBe("anthropic"); - expect(call.model).toBe("claude-opus-4-5"); + expectDefaultSelectedModel(call); }); }); }); @@ -445,35 +453,39 @@ describe("cron model formatting and precedence edge cases", () => { describe("config model format variations", () => { it("default model as string 'provider/model'", async () => { await withTempHome(async (home) => { - const { call } = await runTurn(home, { - cfgOverrides: { - agents: { - defaults: { - model: "openai/gpt-4.1", + await expectTurnModel( + home, + { + cfgOverrides: { + agents: { + defaults: { + model: "openai/gpt-4.1", + }, }, }, + jobPayload: { kind: "agentTurn", message: DEFAULT_MESSAGE, deliver: false }, }, - jobPayload: { kind: "agentTurn", message: DEFAULT_MESSAGE, deliver: false }, - }); - expect(call.provider).toBe("openai"); - expect(call.model).toBe("gpt-4.1"); + { provider: "openai", model: "gpt-4.1" }, + ); }); }); it("default model as object with primary field", async () => { await withTempHome(async (home) => { - const { call } = await runTurn(home, { - cfgOverrides: { - agents: { - defaults: { - model: { primary: "openai/gpt-4.1" }, + await expectTurnModel( + home, + { + cfgOverrides: { + agents: { + defaults: { + model: { primary: "openai/gpt-4.1" }, + }, }, }, + jobPayload: { kind: "agentTurn", message: DEFAULT_MESSAGE, deliver: false }, }, - jobPayload: { kind: "agentTurn", message: DEFAULT_MESSAGE, deliver: false }, - }); - expect(call.provider).toBe("openai"); - expect(call.model).toBe("gpt-4.1"); + { provider: "openai", model: "gpt-4.1" }, + ); }); }); diff --git a/src/cron/isolated-agent/delivery-target.test.ts b/src/cron/isolated-agent/delivery-target.test.ts index cfc492abe3b..df7d29d419f 100644 --- a/src/cron/isolated-agent/delivery-target.test.ts +++ b/src/cron/isolated-agent/delivery-target.test.ts @@ -64,6 +64,23 @@ function setMainSessionEntry(entry?: SessionStore[string]) { vi.mocked(loadSessionStore).mockReturnValue(store); } +function setLastSessionEntry(params: { + sessionId: string; + lastChannel: string; + lastTo: string; + lastThreadId?: string; + lastAccountId?: string; +}) { + setMainSessionEntry({ + sessionId: params.sessionId, + updatedAt: 1000, + lastChannel: params.lastChannel, + lastTo: params.lastTo, + ...(params.lastThreadId ? { lastThreadId: params.lastThreadId } : {}), + ...(params.lastAccountId ? { lastAccountId: params.lastAccountId } : {}), + }); +} + function setWhatsAppAllowFrom(allowFrom: string[]) { vi.mocked(resolveWhatsAppAccount).mockReturnValue({ allowFrom, @@ -86,11 +103,17 @@ async function resolveForAgent(params: { }); } +async function resolveLastTarget(cfg: OpenClawConfig) { + return resolveForAgent({ + cfg, + target: { channel: "last", to: undefined }, + }); +} + describe("resolveDeliveryTarget", () => { it("reroutes implicit whatsapp delivery to authorized allowFrom recipient", async () => { - setMainSessionEntry({ + setLastSessionEntry({ sessionId: "sess-w1", - updatedAt: 1000, lastChannel: "whatsapp", lastTo: "+15550000099", }); @@ -98,16 +121,15 @@ describe("resolveDeliveryTarget", () => { setStoredWhatsAppAllowFrom(["+15550000001"]); const cfg = makeCfg({ bindings: [] }); - const result = await resolveDeliveryTarget(cfg, AGENT_ID, { channel: "last", to: undefined }); + const result = await resolveLastTarget(cfg); expect(result.channel).toBe("whatsapp"); expect(result.to).toBe("+15550000001"); }); it("keeps explicit whatsapp target unchanged", async () => { - setMainSessionEntry({ + setLastSessionEntry({ sessionId: "sess-w2", - updatedAt: 1000, lastChannel: "whatsapp", lastTo: "+15550000099", }); @@ -220,9 +242,8 @@ describe("resolveDeliveryTarget", () => { }); it("drops session threadId when destination does not match the previous recipient", async () => { - setMainSessionEntry({ + setLastSessionEntry({ sessionId: "sess-2", - updatedAt: 1000, lastChannel: "telegram", lastTo: "999999", lastThreadId: "thread-1", @@ -233,9 +254,8 @@ describe("resolveDeliveryTarget", () => { }); it("keeps session threadId when destination matches the previous recipient", async () => { - setMainSessionEntry({ + setLastSessionEntry({ sessionId: "sess-3", - updatedAt: 1000, lastChannel: "telegram", lastTo: "123456", lastThreadId: "thread-2", @@ -248,10 +268,7 @@ describe("resolveDeliveryTarget", () => { it("uses single configured channel when neither explicit nor session channel exists", async () => { setMainSessionEntry(undefined); - const result = await resolveForAgent({ - cfg: makeCfg({ bindings: [] }), - target: { channel: "last", to: undefined }, - }); + const result = await resolveLastTarget(makeCfg({ bindings: [] })); expect(result.channel).toBe("telegram"); expect(result.ok).toBe(false); if (result.ok) { @@ -268,10 +285,7 @@ describe("resolveDeliveryTarget", () => { new Error("Channel is required when multiple channels are configured: telegram, slack"), ); - const result = await resolveForAgent({ - cfg: makeCfg({ bindings: [] }), - target: { channel: "last", to: undefined }, - }); + const result = await resolveLastTarget(makeCfg({ bindings: [] })); expect(result.channel).toBeUndefined(); expect(result.to).toBeUndefined(); expect(result.ok).toBe(false); @@ -308,17 +322,13 @@ describe("resolveDeliveryTarget", () => { }); it("uses main session channel when channel=last and session route exists", async () => { - setMainSessionEntry({ + setLastSessionEntry({ sessionId: "sess-4", - updatedAt: 1000, lastChannel: "telegram", lastTo: "987654", }); - const result = await resolveForAgent({ - cfg: makeCfg({ bindings: [] }), - target: { channel: "last", to: undefined }, - }); + const result = await resolveLastTarget(makeCfg({ bindings: [] })); expect(result.channel).toBe("telegram"); expect(result.to).toBe("987654"); @@ -326,9 +336,8 @@ describe("resolveDeliveryTarget", () => { }); it("explicit delivery.accountId overrides session-derived accountId", async () => { - setMainSessionEntry({ + setLastSessionEntry({ sessionId: "sess-5", - updatedAt: 1000, lastChannel: "telegram", lastTo: "chat-999", lastAccountId: "default", diff --git a/src/cron/isolated-agent/run.interim-retry.test.ts b/src/cron/isolated-agent/run.interim-retry.test.ts index 90d663ed020..6f01a2e9232 100644 --- a/src/cron/isolated-agent/run.interim-retry.test.ts +++ b/src/cron/isolated-agent/run.interim-retry.test.ts @@ -7,6 +7,7 @@ import { countActiveDescendantRunsMock, listDescendantRunsForRequesterMock, loadRunCronIsolatedAgentTurn, + mockRunCronFallbackPassthrough, pickLastNonEmptyTextFromPayloadsMock, runEmbeddedPiAgentMock, runWithModelFallbackMock, @@ -17,13 +18,6 @@ const runCronIsolatedAgentTurn = await loadRunCronIsolatedAgentTurn(); describe("runCronIsolatedAgentTurn — interim ack retry", () => { setupRunCronIsolatedAgentTurnSuite(); - const mockFallbackPassthrough = () => { - runWithModelFallbackMock.mockImplementation(async ({ provider, model, run }) => { - const result = await run(provider, model); - return { result, provider, model, attempts: [] }; - }); - }; - const runTurnAndExpectOk = async (expectedFallbackCalls: number, expectedAgentCalls: number) => { const result = await runCronIsolatedAgentTurn(makeIsolatedAgentTurnParams()); expect(result.status).toBe("ok"); @@ -62,7 +56,7 @@ describe("runCronIsolatedAgentTurn — interim ack retry", () => { meta: { agentMeta: { usage: { input: 10, output: 20 } } }, }); - mockFallbackPassthrough(); + mockRunCronFallbackPassthrough(); await runTurnAndExpectOk(2, 2); expect(runEmbeddedPiAgentMock.mock.calls[1]?.[0]?.prompt).toContain( "previous response was only an acknowledgement", @@ -76,7 +70,7 @@ describe("runCronIsolatedAgentTurn — interim ack retry", () => { meta: { agentMeta: { usage: { input: 10, output: 20 } } }, }); - mockFallbackPassthrough(); + mockRunCronFallbackPassthrough(); await runTurnAndExpectOk(1, 1); }); @@ -93,7 +87,7 @@ describe("runCronIsolatedAgentTurn — interim ack retry", () => { ]); countActiveDescendantRunsMock.mockReturnValue(0); - mockFallbackPassthrough(); + mockRunCronFallbackPassthrough(); await runTurnAndExpectOk(1, 1); }); }); diff --git a/src/cron/isolated-agent/run.message-tool-policy.test.ts b/src/cron/isolated-agent/run.message-tool-policy.test.ts index 2d576900b9d..a92b19f5337 100644 --- a/src/cron/isolated-agent/run.message-tool-policy.test.ts +++ b/src/cron/isolated-agent/run.message-tool-policy.test.ts @@ -2,12 +2,12 @@ import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { clearFastTestEnv, loadRunCronIsolatedAgentTurn, + mockRunCronFallbackPassthrough, resetRunCronIsolatedAgentTurnHarness, resolveCronDeliveryPlanMock, resolveDeliveryTargetMock, restoreFastTestEnv, runEmbeddedPiAgentMock, - runWithModelFallbackMock, } from "./run.test-harness.js"; const runCronIsolatedAgentTurn = await loadRunCronIsolatedAgentTurn(); @@ -32,12 +32,18 @@ function makeParams() { describe("runCronIsolatedAgentTurn message tool policy", () => { let previousFastTestEnv: string | undefined; - const mockFallbackPassthrough = () => { - runWithModelFallbackMock.mockImplementation(async ({ provider, model, run }) => { - const result = await run(provider, model); - return { result, provider, model, attempts: [] }; - }); - }; + async function expectMessageToolDisabledForPlan(plan: { + requested: boolean; + mode: "none" | "announce"; + channel?: string; + to?: string; + }) { + mockRunCronFallbackPassthrough(); + resolveCronDeliveryPlanMock.mockReturnValue(plan); + await runCronIsolatedAgentTurn(makeParams()); + expect(runEmbeddedPiAgentMock).toHaveBeenCalledTimes(1); + expect(runEmbeddedPiAgentMock.mock.calls[0]?.[0]?.disableMessageTool).toBe(true); + } beforeEach(() => { previousFastTestEnv = clearFastTestEnv(); @@ -56,35 +62,23 @@ describe("runCronIsolatedAgentTurn message tool policy", () => { }); it('disables the message tool when delivery.mode is "none"', async () => { - mockFallbackPassthrough(); - resolveCronDeliveryPlanMock.mockReturnValue({ + await expectMessageToolDisabledForPlan({ requested: false, mode: "none", }); - - await runCronIsolatedAgentTurn(makeParams()); - - expect(runEmbeddedPiAgentMock).toHaveBeenCalledTimes(1); - expect(runEmbeddedPiAgentMock.mock.calls[0]?.[0]?.disableMessageTool).toBe(true); }); it("disables the message tool when cron delivery is active", async () => { - mockFallbackPassthrough(); - resolveCronDeliveryPlanMock.mockReturnValue({ + await expectMessageToolDisabledForPlan({ requested: true, mode: "announce", channel: "telegram", to: "123", }); - - await runCronIsolatedAgentTurn(makeParams()); - - expect(runEmbeddedPiAgentMock).toHaveBeenCalledTimes(1); - expect(runEmbeddedPiAgentMock.mock.calls[0]?.[0]?.disableMessageTool).toBe(true); }); it("keeps the message tool enabled for shared callers when delivery is not requested", async () => { - mockFallbackPassthrough(); + mockRunCronFallbackPassthrough(); resolveCronDeliveryPlanMock.mockReturnValue({ requested: false, mode: "none", diff --git a/src/cron/isolated-agent/run.sandbox-config-preserved.test.ts b/src/cron/isolated-agent/run.sandbox-config-preserved.test.ts index 28f3d87cb09..edaee62daa6 100644 --- a/src/cron/isolated-agent/run.sandbox-config-preserved.test.ts +++ b/src/cron/isolated-agent/run.sandbox-config-preserved.test.ts @@ -54,6 +54,31 @@ function makeParams(overrides?: Record) { }; } +function expectDefaultSandboxPreserved( + runCfg: + | { + agents?: { defaults?: { sandbox?: unknown } }; + } + | undefined, +) { + expect(runCfg?.agents?.defaults?.sandbox).toEqual({ + mode: "all", + workspaceAccess: "rw", + docker: { + network: "none", + dangerouslyAllowContainerNamespaceJoin: true, + dangerouslyAllowExternalBindSources: true, + }, + browser: { + enabled: true, + autoStart: false, + }, + prune: { + maxAgeDays: 7, + }, + }); +} + describe("runCronIsolatedAgentTurn sandbox config preserved", () => { let previousFastTestEnv: string | undefined; @@ -79,22 +104,7 @@ describe("runCronIsolatedAgentTurn sandbox config preserved", () => { expect(runWithModelFallbackMock).toHaveBeenCalledTimes(1); const runCfg = runWithModelFallbackMock.mock.calls[0]?.[0]?.cfg; - expect(runCfg?.agents?.defaults?.sandbox).toEqual({ - mode: "all", - workspaceAccess: "rw", - docker: { - network: "none", - dangerouslyAllowContainerNamespaceJoin: true, - dangerouslyAllowExternalBindSources: true, - }, - browser: { - enabled: true, - autoStart: false, - }, - prune: { - maxAgeDays: 7, - }, - }); + expectDefaultSandboxPreserved(runCfg); }); it("keeps global sandbox defaults when agent override is partial", async () => { @@ -118,22 +128,7 @@ describe("runCronIsolatedAgentTurn sandbox config preserved", () => { const runCfg = runWithModelFallbackMock.mock.calls[0]?.[0]?.cfg; const resolvedSandbox = resolveSandboxConfigForAgent(runCfg, "specialist"); - expect(runCfg?.agents?.defaults?.sandbox).toEqual({ - mode: "all", - workspaceAccess: "rw", - docker: { - network: "none", - dangerouslyAllowContainerNamespaceJoin: true, - dangerouslyAllowExternalBindSources: true, - }, - browser: { - enabled: true, - autoStart: false, - }, - prune: { - maxAgeDays: 7, - }, - }); + expectDefaultSandboxPreserved(runCfg); expect(resolvedSandbox.mode).toBe("all"); expect(resolvedSandbox.workspaceAccess).toBe("rw"); expect(resolvedSandbox.docker).toMatchObject({ diff --git a/src/cron/isolated-agent/run.test-harness.ts b/src/cron/isolated-agent/run.test-harness.ts index 74b5eed43e1..81e4c8b902b 100644 --- a/src/cron/isolated-agent/run.test-harness.ts +++ b/src/cron/isolated-agent/run.test-harness.ts @@ -341,6 +341,13 @@ function makeDefaultEmbeddedResult() { }; } +export function mockRunCronFallbackPassthrough(): void { + runWithModelFallbackMock.mockImplementation(async ({ provider, model, run }) => { + const result = await run(provider, model); + return { result, provider, model, attempts: [] }; + }); +} + export function resetRunCronIsolatedAgentTurnHarness(): void { vi.clearAllMocks(); diff --git a/src/cron/isolated-agent/subagent-followup.test.ts b/src/cron/isolated-agent/subagent-followup.test.ts index c670e4c8c13..7861c75ff35 100644 --- a/src/cron/isolated-agent/subagent-followup.test.ts +++ b/src/cron/isolated-agent/subagent-followup.test.ts @@ -33,6 +33,29 @@ async function resolveAfterAdvancingTimers(promise: Promise, advanceMs = 1 return promise; } +function createDescendantRun(params?: { + runId?: string; + childSessionKey?: string; + task?: string; + cleanup?: "keep" | "delete"; + endedAt?: number; + frozenResultText?: string | null; +}) { + return { + runId: params?.runId ?? "run-1", + childSessionKey: params?.childSessionKey ?? "child-1", + requesterSessionKey: "test-session", + requesterDisplayKey: "test-session", + task: params?.task ?? "task-1", + cleanup: params?.cleanup ?? "keep", + createdAt: 1000, + endedAt: params?.endedAt ?? 2000, + ...(params?.frozenResultText === undefined + ? {} + : { frozenResultText: params.frozenResultText }), + }; +} + describe("isLikelyInterimCronMessage", () => { it("detects 'on it' as interim", () => { expect(isLikelyInterimCronMessage("on it")).toBe(true); @@ -85,18 +108,7 @@ describe("readDescendantSubagentFallbackReply", () => { }); it("reads reply from child session transcript", async () => { - vi.mocked(listDescendantRunsForRequester).mockReturnValue([ - { - runId: "run-1", - childSessionKey: "child-1", - requesterSessionKey: "test-session", - requesterDisplayKey: "test-session", - task: "task-1", - cleanup: "keep", - createdAt: 1000, - endedAt: 2000, - }, - ]); + vi.mocked(listDescendantRunsForRequester).mockReturnValue([createDescendantRun()]); vi.mocked(readLatestAssistantReply).mockResolvedValue("child output text"); const result = await readDescendantSubagentFallbackReply({ sessionKey: "test-session", @@ -107,17 +119,10 @@ describe("readDescendantSubagentFallbackReply", () => { it("falls back to frozenResultText when session transcript unavailable", async () => { vi.mocked(listDescendantRunsForRequester).mockReturnValue([ - { - runId: "run-1", - childSessionKey: "child-1", - requesterSessionKey: "test-session", - requesterDisplayKey: "test-session", - task: "task-1", + createDescendantRun({ cleanup: "delete", - createdAt: 1000, - endedAt: 2000, frozenResultText: "frozen child output", - }, + }), ]); vi.mocked(readLatestAssistantReply).mockResolvedValue(undefined); const result = await readDescendantSubagentFallbackReply({ @@ -129,17 +134,7 @@ describe("readDescendantSubagentFallbackReply", () => { it("prefers session transcript over frozenResultText", async () => { vi.mocked(listDescendantRunsForRequester).mockReturnValue([ - { - runId: "run-1", - childSessionKey: "child-1", - requesterSessionKey: "test-session", - requesterDisplayKey: "test-session", - task: "task-1", - cleanup: "keep", - createdAt: 1000, - endedAt: 2000, - frozenResultText: "frozen text", - }, + createDescendantRun({ frozenResultText: "frozen text" }), ]); vi.mocked(readLatestAssistantReply).mockResolvedValue("live transcript text"); const result = await readDescendantSubagentFallbackReply({ @@ -151,28 +146,14 @@ describe("readDescendantSubagentFallbackReply", () => { it("joins replies from multiple descendants", async () => { vi.mocked(listDescendantRunsForRequester).mockReturnValue([ - { - runId: "run-1", - childSessionKey: "child-1", - requesterSessionKey: "test-session", - requesterDisplayKey: "test-session", - task: "task-1", - cleanup: "keep", - createdAt: 1000, - endedAt: 2000, - frozenResultText: "first child output", - }, - { + createDescendantRun({ frozenResultText: "first child output" }), + createDescendantRun({ runId: "run-2", childSessionKey: "child-2", - requesterSessionKey: "test-session", - requesterDisplayKey: "test-session", task: "task-2", - cleanup: "keep", - createdAt: 1000, endedAt: 3000, frozenResultText: "second child output", - }, + }), ]); vi.mocked(readLatestAssistantReply).mockResolvedValue(undefined); const result = await readDescendantSubagentFallbackReply({ @@ -184,27 +165,14 @@ describe("readDescendantSubagentFallbackReply", () => { it("skips SILENT_REPLY_TOKEN descendants", async () => { vi.mocked(listDescendantRunsForRequester).mockReturnValue([ - { - runId: "run-1", - childSessionKey: "child-1", - requesterSessionKey: "test-session", - requesterDisplayKey: "test-session", - task: "task-1", - cleanup: "keep", - createdAt: 1000, - endedAt: 2000, - }, - { + createDescendantRun(), + createDescendantRun({ runId: "run-2", childSessionKey: "child-2", - requesterSessionKey: "test-session", - requesterDisplayKey: "test-session", task: "task-2", - cleanup: "keep", - createdAt: 1000, endedAt: 3000, frozenResultText: "useful output", - }, + }), ]); vi.mocked(readLatestAssistantReply).mockImplementation(async (params) => { if (params.sessionKey === "child-1") { @@ -221,17 +189,10 @@ describe("readDescendantSubagentFallbackReply", () => { it("returns undefined when frozenResultText is null", async () => { vi.mocked(listDescendantRunsForRequester).mockReturnValue([ - { - runId: "run-1", - childSessionKey: "child-1", - requesterSessionKey: "test-session", - requesterDisplayKey: "test-session", - task: "task-1", + createDescendantRun({ cleanup: "delete", - createdAt: 1000, - endedAt: 2000, frozenResultText: null, - }, + }), ]); vi.mocked(readLatestAssistantReply).mockResolvedValue(undefined); const result = await readDescendantSubagentFallbackReply({ diff --git a/src/cron/isolated-agent/subagent-followup.ts b/src/cron/isolated-agent/subagent-followup.ts index 9d6ec7e78ac..a337fe528b7 100644 --- a/src/cron/isolated-agent/subagent-followup.ts +++ b/src/cron/isolated-agent/subagent-followup.ts @@ -169,7 +169,7 @@ export async function waitForDescendantSubagentSummary(params: { // CRON_SUBAGENT_FINAL_REPLY_GRACE_MS) to capture that synthesis. const gracePeriodDeadline = Math.min(Date.now() + CRON_SUBAGENT_FINAL_REPLY_GRACE_MS, deadline); - while (Date.now() < gracePeriodDeadline) { + const resolveUsableLatestReply = async () => { const latest = (await readLatestAssistantReply({ sessionKey: params.sessionKey }))?.trim(); if ( latest && @@ -178,16 +178,20 @@ export async function waitForDescendantSubagentSummary(params: { ) { return latest; } + return undefined; + }; + + while (Date.now() < gracePeriodDeadline) { + const latest = await resolveUsableLatestReply(); + if (latest) { + return latest; + } await new Promise((resolve) => setTimeout(resolve, CRON_SUBAGENT_GRACE_POLL_MS)); } // Final read after grace period expires. - const latest = (await readLatestAssistantReply({ sessionKey: params.sessionKey }))?.trim(); - if ( - latest && - latest.toUpperCase() !== SILENT_REPLY_TOKEN.toUpperCase() && - (latest !== initialReply || !isLikelyInterimCronMessage(latest)) - ) { + const latest = await resolveUsableLatestReply(); + if (latest) { return latest; } diff --git a/src/daemon/schtasks.startup-fallback.test.ts b/src/daemon/schtasks.startup-fallback.test.ts index 6e6a8521d6c..4fdadd2f110 100644 --- a/src/daemon/schtasks.startup-fallback.test.ts +++ b/src/daemon/schtasks.startup-fallback.test.ts @@ -57,6 +57,30 @@ async function writeGatewayScript(env: Record, port = 18789) { "utf8", ); } +async function writeStartupFallbackEntry(env: Record) { + const startupEntryPath = resolveStartupEntryPath(env); + await fs.mkdir(path.dirname(startupEntryPath), { recursive: true }); + await fs.writeFile(startupEntryPath, "@echo off\r\n", "utf8"); + return startupEntryPath; +} + +function expectStartupFallbackSpawn(env: Record) { + expect(spawn).toHaveBeenCalledWith( + "cmd.exe", + ["/d", "/s", "/c", quoteCmdScriptArg(resolveTaskScriptPath(env))], + expect.objectContaining({ detached: true, stdio: "ignore", windowsHide: true }), + ); +} + +function addStartupFallbackMissingResponses( + extraResponses: Array<{ code: number; stdout: string; stderr: string }> = [], +) { + schtasksResponses.push( + { code: 0, stdout: "", stderr: "" }, + { code: 1, stdout: "", stderr: "not found" }, + ...extraResponses, + ); +} beforeEach(() => { resetSchtasksBaseMocks(); spawn.mockClear(); @@ -119,22 +143,14 @@ describe("Windows startup fallback", () => { }); await expect(fs.access(resolveStartupEntryPath(env))).resolves.toBeUndefined(); - expect(spawn).toHaveBeenCalledWith( - "cmd.exe", - ["/d", "/s", "/c", quoteCmdScriptArg(resolveTaskScriptPath(env))], - expect.objectContaining({ detached: true, stdio: "ignore", windowsHide: true }), - ); + expectStartupFallbackSpawn(env); }); }); it("treats an installed Startup-folder launcher as loaded", async () => { await withWindowsEnv("openclaw-win-startup-", async ({ env }) => { - schtasksResponses.push( - { code: 0, stdout: "", stderr: "" }, - { code: 1, stdout: "", stderr: "not found" }, - ); - await fs.mkdir(path.dirname(resolveStartupEntryPath(env)), { recursive: true }); - await fs.writeFile(resolveStartupEntryPath(env), "@echo off\r\n", "utf8"); + addStartupFallbackMissingResponses(); + await writeStartupFallbackEntry(env); await expect(isScheduledTaskInstalled({ env })).resolves.toBe(true); }); @@ -142,12 +158,8 @@ describe("Windows startup fallback", () => { it("reports runtime from the gateway listener when using the Startup fallback", async () => { await withWindowsEnv("openclaw-win-startup-", async ({ env }) => { - schtasksResponses.push( - { code: 0, stdout: "", stderr: "" }, - { code: 1, stdout: "", stderr: "not found" }, - ); - await fs.mkdir(path.dirname(resolveStartupEntryPath(env)), { recursive: true }); - await fs.writeFile(resolveStartupEntryPath(env), "@echo off\r\n", "utf8"); + addStartupFallbackMissingResponses(); + await writeStartupFallbackEntry(env); inspectPortUsage.mockResolvedValue({ port: 18789, status: "busy", @@ -164,14 +176,11 @@ describe("Windows startup fallback", () => { it("restarts the Startup fallback by killing the current pid and relaunching the entry", async () => { await withWindowsEnv("openclaw-win-startup-", async ({ env }) => { - schtasksResponses.push( + addStartupFallbackMissingResponses([ { code: 0, stdout: "", stderr: "" }, { code: 1, stdout: "", stderr: "not found" }, - { code: 0, stdout: "", stderr: "" }, - { code: 1, stdout: "", stderr: "not found" }, - ); - await fs.mkdir(path.dirname(resolveStartupEntryPath(env)), { recursive: true }); - await fs.writeFile(resolveStartupEntryPath(env), "@echo off\r\n", "utf8"); + ]); + await writeStartupFallbackEntry(env); inspectPortUsage.mockResolvedValue({ port: 18789, status: "busy", @@ -184,11 +193,7 @@ describe("Windows startup fallback", () => { outcome: "completed", }); expect(killProcessTree).toHaveBeenCalledWith(5151, { graceMs: 300 }); - expect(spawn).toHaveBeenCalledWith( - "cmd.exe", - ["/d", "/s", "/c", quoteCmdScriptArg(resolveTaskScriptPath(env))], - expect.objectContaining({ detached: true, stdio: "ignore", windowsHide: true }), - ); + expectStartupFallbackSpawn(env); }); }); @@ -196,8 +201,7 @@ describe("Windows startup fallback", () => { await withWindowsEnv("openclaw-win-startup-", async ({ env }) => { schtasksResponses.push({ code: 0, stdout: "", stderr: "" }); await writeGatewayScript(env); - await fs.mkdir(path.dirname(resolveStartupEntryPath(env)), { recursive: true }); - await fs.writeFile(resolveStartupEntryPath(env), "@echo off\r\n", "utf8"); + await writeStartupFallbackEntry(env); inspectPortUsage .mockResolvedValueOnce({ port: 18789, diff --git a/src/discord/monitor/auto-presence.test.ts b/src/discord/monitor/auto-presence.test.ts index b5a83d5242d..d901a76d642 100644 --- a/src/discord/monitor/auto-presence.test.ts +++ b/src/discord/monitor/auto-presence.test.ts @@ -29,45 +29,33 @@ function createStore(params?: { }; } +function expectExhaustedDecision(params: { failureCounts: Record }) { + const now = Date.now(); + const decision = resolveDiscordAutoPresenceDecision({ + discordConfig: { + autoPresence: { + enabled: true, + exhaustedText: "token exhausted", + }, + }, + authStore: createStore({ cooldownUntil: now + 60_000, failureCounts: params.failureCounts }), + gatewayConnected: true, + now, + }); + + expect(decision).toBeTruthy(); + expect(decision?.state).toBe("exhausted"); + expect(decision?.presence.status).toBe("dnd"); + expect(decision?.presence.activities[0]?.state).toBe("token exhausted"); +} + describe("discord auto presence", () => { it("maps exhausted runtime signal to dnd", () => { - const now = Date.now(); - const decision = resolveDiscordAutoPresenceDecision({ - discordConfig: { - autoPresence: { - enabled: true, - exhaustedText: "token exhausted", - }, - }, - authStore: createStore({ cooldownUntil: now + 60_000, failureCounts: { rate_limit: 2 } }), - gatewayConnected: true, - now, - }); - - expect(decision).toBeTruthy(); - expect(decision?.state).toBe("exhausted"); - expect(decision?.presence.status).toBe("dnd"); - expect(decision?.presence.activities[0]?.state).toBe("token exhausted"); + expectExhaustedDecision({ failureCounts: { rate_limit: 2 } }); }); it("treats overloaded cooldown as exhausted", () => { - const now = Date.now(); - const decision = resolveDiscordAutoPresenceDecision({ - discordConfig: { - autoPresence: { - enabled: true, - exhaustedText: "token exhausted", - }, - }, - authStore: createStore({ cooldownUntil: now + 60_000, failureCounts: { overloaded: 2 } }), - gatewayConnected: true, - now, - }); - - expect(decision).toBeTruthy(); - expect(decision?.state).toBe("exhausted"); - expect(decision?.presence.status).toBe("dnd"); - expect(decision?.presence.activities[0]?.state).toBe("token exhausted"); + expectExhaustedDecision({ failureCounts: { overloaded: 2 } }); }); it("recovers from exhausted to online once a profile becomes usable", () => { diff --git a/src/docker-setup.e2e.test.ts b/src/docker-setup.e2e.test.ts index 8d5eec70ed0..160c74b8a21 100644 --- a/src/docker-setup.e2e.test.ts +++ b/src/docker-setup.e2e.test.ts @@ -113,6 +113,26 @@ function runDockerSetup( }); } +async function runDockerSetupWithUnsetGatewayToken( + sandbox: DockerSetupSandbox, + suffix: string, + prepare?: (configDir: string) => Promise, +) { + const configDir = join(sandbox.rootDir, `config-${suffix}`); + const workspaceDir = join(sandbox.rootDir, `workspace-${suffix}`); + await mkdir(configDir, { recursive: true }); + await prepare?.(configDir); + + const result = runDockerSetup(sandbox, { + OPENCLAW_GATEWAY_TOKEN: undefined, + OPENCLAW_CONFIG_DIR: configDir, + OPENCLAW_WORKSPACE_DIR: workspaceDir, + }); + const envFile = await readFile(join(sandbox.rootDir, ".env"), "utf8"); + + return { result, envFile }; +} + async function withUnixSocket(socketPath: string, run: () => Promise): Promise { const server = createServer(); await new Promise((resolve, reject) => { @@ -243,52 +263,39 @@ describe("docker-setup.sh", () => { it("reuses existing config token when OPENCLAW_GATEWAY_TOKEN is unset", async () => { const activeSandbox = requireSandbox(sandbox); - const configDir = join(activeSandbox.rootDir, "config-token-reuse"); - const workspaceDir = join(activeSandbox.rootDir, "workspace-token-reuse"); - await mkdir(configDir, { recursive: true }); - await writeFile( - join(configDir, "openclaw.json"), - JSON.stringify({ gateway: { auth: { mode: "token", token: "config-token-123" } } }), + const { result, envFile } = await runDockerSetupWithUnsetGatewayToken( + activeSandbox, + "token-reuse", + async (configDir) => { + await writeFile( + join(configDir, "openclaw.json"), + JSON.stringify({ gateway: { auth: { mode: "token", token: "config-token-123" } } }), + ); + }, ); - const result = runDockerSetup(activeSandbox, { - OPENCLAW_GATEWAY_TOKEN: undefined, - OPENCLAW_CONFIG_DIR: configDir, - OPENCLAW_WORKSPACE_DIR: workspaceDir, - }); - expect(result.status).toBe(0); - const envFile = await readFile(join(activeSandbox.rootDir, ".env"), "utf8"); expect(envFile).toContain("OPENCLAW_GATEWAY_TOKEN=config-token-123"); // pragma: allowlist secret }); it("reuses existing .env token when OPENCLAW_GATEWAY_TOKEN and config token are unset", async () => { const activeSandbox = requireSandbox(sandbox); - const configDir = join(activeSandbox.rootDir, "config-dotenv-token-reuse"); - const workspaceDir = join(activeSandbox.rootDir, "workspace-dotenv-token-reuse"); - await mkdir(configDir, { recursive: true }); await writeFile( join(activeSandbox.rootDir, ".env"), "OPENCLAW_GATEWAY_TOKEN=dotenv-token-123\nOPENCLAW_GATEWAY_PORT=18789\n", // pragma: allowlist secret ); - - const result = runDockerSetup(activeSandbox, { - OPENCLAW_GATEWAY_TOKEN: undefined, - OPENCLAW_CONFIG_DIR: configDir, - OPENCLAW_WORKSPACE_DIR: workspaceDir, - }); + const { result, envFile } = await runDockerSetupWithUnsetGatewayToken( + activeSandbox, + "dotenv-token-reuse", + ); expect(result.status).toBe(0); - const envFile = await readFile(join(activeSandbox.rootDir, ".env"), "utf8"); expect(envFile).toContain("OPENCLAW_GATEWAY_TOKEN=dotenv-token-123"); // pragma: allowlist secret expect(result.stderr).toBe(""); }); it("reuses the last non-empty .env token and strips CRLF without truncating '='", async () => { const activeSandbox = requireSandbox(sandbox); - const configDir = join(activeSandbox.rootDir, "config-dotenv-last-wins"); - const workspaceDir = join(activeSandbox.rootDir, "workspace-dotenv-last-wins"); - await mkdir(configDir, { recursive: true }); await writeFile( join(activeSandbox.rootDir, ".env"), [ @@ -297,15 +304,12 @@ describe("docker-setup.sh", () => { "OPENCLAW_GATEWAY_TOKEN=last=token=value\r", // pragma: allowlist secret ].join("\n"), ); - - const result = runDockerSetup(activeSandbox, { - OPENCLAW_GATEWAY_TOKEN: undefined, - OPENCLAW_CONFIG_DIR: configDir, - OPENCLAW_WORKSPACE_DIR: workspaceDir, - }); + const { result, envFile } = await runDockerSetupWithUnsetGatewayToken( + activeSandbox, + "dotenv-last-wins", + ); expect(result.status).toBe(0); - const envFile = await readFile(join(activeSandbox.rootDir, ".env"), "utf8"); expect(envFile).toContain("OPENCLAW_GATEWAY_TOKEN=last=token=value"); // pragma: allowlist secret expect(envFile).not.toContain("OPENCLAW_GATEWAY_TOKEN=first-token"); expect(envFile).not.toContain("\r"); diff --git a/src/gateway/server-cron.test.ts b/src/gateway/server-cron.test.ts index 945840a7106..2608560e20f 100644 --- a/src/gateway/server-cron.test.ts +++ b/src/gateway/server-cron.test.ts @@ -10,12 +10,20 @@ const requestHeartbeatNowMock = vi.fn(); const loadConfigMock = vi.fn(); const fetchWithSsrFGuardMock = vi.fn(); +function enqueueSystemEvent(...args: unknown[]) { + return enqueueSystemEventMock(...args); +} + +function requestHeartbeatNow(...args: unknown[]) { + return requestHeartbeatNowMock(...args); +} + vi.mock("../infra/system-events.js", () => ({ - enqueueSystemEvent: (...args: unknown[]) => enqueueSystemEventMock(...args), + enqueueSystemEvent, })); vi.mock("../infra/heartbeat-wake.js", () => ({ - requestHeartbeatNow: (...args: unknown[]) => requestHeartbeatNowMock(...args), + requestHeartbeatNow, })); vi.mock("../config/config.js", async () => { @@ -32,6 +40,18 @@ vi.mock("../infra/net/fetch-guard.js", () => ({ import { buildGatewayCronService } from "./server-cron.js"; +function createCronConfig(name: string): OpenClawConfig { + const tmpDir = path.join(os.tmpdir(), `${name}-${Date.now()}`); + return { + session: { + mainKey: "main", + }, + cron: { + store: path.join(tmpDir, "cron.json"), + }, + } as OpenClawConfig; +} + describe("buildGatewayCronService", () => { beforeEach(() => { enqueueSystemEventMock.mockClear(); @@ -41,15 +61,7 @@ describe("buildGatewayCronService", () => { }); it("routes main-target jobs to the scoped session for enqueue + wake", async () => { - const tmpDir = path.join(os.tmpdir(), `server-cron-${Date.now()}`); - const cfg = { - session: { - mainKey: "main", - }, - cron: { - store: path.join(tmpDir, "cron.json"), - }, - } as OpenClawConfig; + const cfg = createCronConfig("server-cron"); loadConfigMock.mockReturnValue(cfg); const state = buildGatewayCronService({ @@ -87,16 +99,7 @@ describe("buildGatewayCronService", () => { }); it("blocks private webhook URLs via SSRF-guarded fetch", async () => { - const tmpDir = path.join(os.tmpdir(), `server-cron-ssrf-${Date.now()}`); - const cfg = { - session: { - mainKey: "main", - }, - cron: { - store: path.join(tmpDir, "cron.json"), - }, - } as OpenClawConfig; - + const cfg = createCronConfig("server-cron-ssrf"); loadConfigMock.mockReturnValue(cfg); fetchWithSsrFGuardMock.mockRejectedValue( new SsrFBlockedError("Blocked: resolves to private/internal/special-use IP address"), diff --git a/src/gateway/server-methods/agent-wait-dedupe.ts b/src/gateway/server-methods/agent-wait-dedupe.ts index 98d0df72fa3..50629beb3eb 100644 --- a/src/gateway/server-methods/agent-wait-dedupe.ts +++ b/src/gateway/server-methods/agent-wait-dedupe.ts @@ -23,6 +23,17 @@ function asFiniteNumber(value: unknown): number | undefined { return typeof value === "number" && Number.isFinite(value) ? value : undefined; } +function removeWaiter(runId: string, waiter: () => void): void { + const waiters = AGENT_WAITERS_BY_RUN_ID.get(runId); + if (!waiters) { + return; + } + waiters.delete(waiter); + if (waiters.size === 0) { + AGENT_WAITERS_BY_RUN_ID.delete(runId); + } +} + function addWaiter(runId: string, waiter: () => void): () => void { const normalizedRunId = runId.trim(); if (!normalizedRunId) { @@ -31,28 +42,10 @@ function addWaiter(runId: string, waiter: () => void): () => void { const existing = AGENT_WAITERS_BY_RUN_ID.get(normalizedRunId); if (existing) { existing.add(waiter); - return () => { - const waiters = AGENT_WAITERS_BY_RUN_ID.get(normalizedRunId); - if (!waiters) { - return; - } - waiters.delete(waiter); - if (waiters.size === 0) { - AGENT_WAITERS_BY_RUN_ID.delete(normalizedRunId); - } - }; + return () => removeWaiter(normalizedRunId, waiter); } AGENT_WAITERS_BY_RUN_ID.set(normalizedRunId, new Set([waiter])); - return () => { - const waiters = AGENT_WAITERS_BY_RUN_ID.get(normalizedRunId); - if (!waiters) { - return; - } - waiters.delete(waiter); - if (waiters.size === 0) { - AGENT_WAITERS_BY_RUN_ID.delete(normalizedRunId); - } - }; + return () => removeWaiter(normalizedRunId, waiter); } function notifyWaiters(runId: string): void { diff --git a/src/gateway/server/ws-connection/handshake-auth-helpers.test.ts b/src/gateway/server/ws-connection/handshake-auth-helpers.test.ts index 68ec4e1a153..8b7b7e521fd 100644 --- a/src/gateway/server/ws-connection/handshake-auth-helpers.test.ts +++ b/src/gateway/server/ws-connection/handshake-auth-helpers.test.ts @@ -10,24 +10,21 @@ import { shouldSkipBackendSelfPairing, } from "./handshake-auth-helpers.js"; +function createRateLimiter(): AuthRateLimiter { + return { + check: () => ({ allowed: true, remaining: 1, retryAfterMs: 0 }), + reset: () => {}, + recordFailure: () => {}, + size: () => 0, + prune: () => {}, + dispose: () => {}, + }; +} + describe("handshake auth helpers", () => { it("pins browser-origin loopback clients to the synthetic rate-limit ip", () => { - const rateLimiter: AuthRateLimiter = { - check: () => ({ allowed: true, remaining: 1, retryAfterMs: 0 }), - reset: () => {}, - recordFailure: () => {}, - size: () => 0, - prune: () => {}, - dispose: () => {}, - }; - const browserRateLimiter: AuthRateLimiter = { - check: () => ({ allowed: true, remaining: 1, retryAfterMs: 0 }), - reset: () => {}, - recordFailure: () => {}, - size: () => 0, - prune: () => {}, - dispose: () => {}, - }; + const rateLimiter = createRateLimiter(); + const browserRateLimiter = createRateLimiter(); const resolved = resolveHandshakeBrowserSecurityContext({ requestOrigin: "https://app.example", clientIp: "127.0.0.1", diff --git a/src/gateway/server/ws-connection/handshake-auth-helpers.ts b/src/gateway/server/ws-connection/handshake-auth-helpers.ts index cce5b979b3e..8529cf55082 100644 --- a/src/gateway/server/ws-connection/handshake-auth-helpers.ts +++ b/src/gateway/server/ws-connection/handshake-auth-helpers.ts @@ -91,6 +91,23 @@ function resolveSignatureToken(connectParams: ConnectParams): string | null { ); } +function buildUnauthorizedHandshakeContext(params: { + authProvided: AuthProvidedKind; + canRetryWithDeviceToken: boolean; + recommendedNextStep: + | "retry_with_device_token" + | "update_auth_configuration" + | "update_auth_credentials" + | "wait_then_retry" + | "review_auth_configuration"; +}) { + return { + authProvided: params.authProvided, + canRetryWithDeviceToken: params.canRetryWithDeviceToken, + recommendedNextStep: params.recommendedNextStep, + }; +} + export function resolveDeviceSignaturePayloadVersion(params: { device: { id: string; @@ -104,7 +121,7 @@ export function resolveDeviceSignaturePayloadVersion(params: { nonce: string; }): "v3" | "v2" | null { const signatureToken = resolveSignatureToken(params.connectParams); - const payloadV3 = buildDeviceAuthPayloadV3({ + const basePayload = { deviceId: params.device.id, clientId: params.connectParams.client.id, clientMode: params.connectParams.client.mode, @@ -113,6 +130,9 @@ export function resolveDeviceSignaturePayloadVersion(params: { signedAtMs: params.signedAtMs, token: signatureToken, nonce: params.nonce, + }; + const payloadV3 = buildDeviceAuthPayloadV3({ + ...basePayload, platform: params.connectParams.client.platform, deviceFamily: params.connectParams.client.deviceFamily, }); @@ -120,16 +140,7 @@ export function resolveDeviceSignaturePayloadVersion(params: { return "v3"; } - const payloadV2 = buildDeviceAuthPayload({ - deviceId: params.device.id, - clientId: params.connectParams.client.id, - clientMode: params.connectParams.client.mode, - role: params.role, - scopes: params.scopes, - signedAtMs: params.signedAtMs, - token: signatureToken, - nonce: params.nonce, - }); + const payloadV2 = buildDeviceAuthPayload(basePayload); if (verifyDeviceSignature(params.device.publicKey, payloadV2, params.device.signature)) { return "v2"; } @@ -171,41 +182,41 @@ export function resolveUnauthorizedHandshakeContext(params: { authProvided === "token" && !params.connectAuth?.deviceToken; if (canRetryWithDeviceToken) { - return { + return buildUnauthorizedHandshakeContext({ authProvided, canRetryWithDeviceToken, recommendedNextStep: "retry_with_device_token", - }; + }); } switch (params.failedAuth.reason) { case "token_missing": case "token_missing_config": case "password_missing": case "password_missing_config": - return { + return buildUnauthorizedHandshakeContext({ authProvided, canRetryWithDeviceToken, recommendedNextStep: "update_auth_configuration", - }; + }); case "token_mismatch": case "password_mismatch": case "device_token_mismatch": - return { + return buildUnauthorizedHandshakeContext({ authProvided, canRetryWithDeviceToken, recommendedNextStep: "update_auth_credentials", - }; + }); case "rate_limited": - return { + return buildUnauthorizedHandshakeContext({ authProvided, canRetryWithDeviceToken, recommendedNextStep: "wait_then_retry", - }; + }); default: - return { + return buildUnauthorizedHandshakeContext({ authProvided, canRetryWithDeviceToken, recommendedNextStep: "review_auth_configuration", - }; + }); } } diff --git a/src/infra/heartbeat-wake.test.ts b/src/infra/heartbeat-wake.test.ts index 1f800c655ed..5a88210505c 100644 --- a/src/infra/heartbeat-wake.test.ts +++ b/src/infra/heartbeat-wake.test.ts @@ -8,6 +8,15 @@ import { } from "./heartbeat-wake.js"; describe("heartbeat-wake", () => { + function setRetryOnceHeartbeatHandler() { + const handler = vi + .fn() + .mockResolvedValueOnce({ status: "skipped", reason: "requests-in-flight" }) + .mockResolvedValueOnce({ status: "ran", durationMs: 1 }); + setHeartbeatWakeHandler(handler); + return handler; + } + async function expectRetryAfterDefaultDelay(params: { handler: ReturnType; initialReason: string; @@ -74,11 +83,7 @@ describe("heartbeat-wake", () => { it("keeps retry cooldown even when a sooner request arrives", async () => { vi.useFakeTimers(); - const handler = vi - .fn() - .mockResolvedValueOnce({ status: "skipped", reason: "requests-in-flight" }) - .mockResolvedValueOnce({ status: "ran", durationMs: 1 }); - setHeartbeatWakeHandler(handler); + const handler = setRetryOnceHeartbeatHandler(); requestHeartbeatNow({ reason: "interval", coalesceMs: 0 }); await vi.advanceTimersByTimeAsync(1); @@ -252,11 +257,7 @@ describe("heartbeat-wake", () => { it("forwards wake target fields and preserves them across retries", async () => { vi.useFakeTimers(); - const handler = vi - .fn() - .mockResolvedValueOnce({ status: "skipped", reason: "requests-in-flight" }) - .mockResolvedValueOnce({ status: "ran", durationMs: 1 }); - setHeartbeatWakeHandler(handler); + const handler = setRetryOnceHeartbeatHandler(); requestHeartbeatNow({ reason: "cron:job-1", diff --git a/src/infra/home-dir.ts b/src/infra/home-dir.ts index 7dd2bbdd1ec..650cf0cadac 100644 --- a/src/infra/home-dir.ts +++ b/src/infra/home-dir.ts @@ -75,3 +75,25 @@ export function expandHomePrefix( } return input.replace(/^~(?=$|[\\/])/, home); } + +export function resolveHomeRelativePath( + input: string, + opts?: { + env?: NodeJS.ProcessEnv; + homedir?: () => string; + }, +): string { + const trimmed = input.trim(); + if (!trimmed) { + return trimmed; + } + if (trimmed.startsWith("~")) { + const expanded = expandHomePrefix(trimmed, { + home: resolveRequiredHomeDir(opts?.env ?? process.env, opts?.homedir ?? os.homedir), + env: opts?.env, + homedir: opts?.homedir, + }); + return path.resolve(expanded); + } + return path.resolve(trimmed); +} diff --git a/src/infra/host-env-security.ts b/src/infra/host-env-security.ts index 8c5d0989fdd..11d6b8e9f3c 100644 --- a/src/infra/host-env-security.ts +++ b/src/infra/host-env-security.ts @@ -80,6 +80,23 @@ export function isDangerousHostEnvOverrideVarName(rawKey: string): boolean { return HOST_DANGEROUS_OVERRIDE_ENV_PREFIXES.some((prefix) => upper.startsWith(prefix)); } +function listNormalizedPortableEnvEntries( + source: Record, +): Array<[string, string]> { + const entries: Array<[string, string]> = []; + for (const [rawKey, value] of Object.entries(source)) { + if (typeof value !== "string") { + continue; + } + const key = normalizeEnvVarKey(rawKey, { portable: true }); + if (!key) { + continue; + } + entries.push([key, value]); + } + return entries; +} + export function sanitizeHostExecEnv(params?: { baseEnv?: Record; overrides?: Record | null; @@ -90,12 +107,8 @@ export function sanitizeHostExecEnv(params?: { const blockPathOverrides = params?.blockPathOverrides ?? true; const merged: Record = {}; - for (const [rawKey, value] of Object.entries(baseEnv)) { - if (typeof value !== "string") { - continue; - } - const key = normalizeEnvVarKey(rawKey, { portable: true }); - if (!key || isDangerousHostEnvVarName(key)) { + for (const [key, value] of listNormalizedPortableEnvEntries(baseEnv)) { + if (isDangerousHostEnvVarName(key)) { continue; } merged[key] = value; @@ -105,14 +118,7 @@ export function sanitizeHostExecEnv(params?: { return markOpenClawExecEnv(merged); } - for (const [rawKey, value] of Object.entries(overrides)) { - if (typeof value !== "string") { - continue; - } - const key = normalizeEnvVarKey(rawKey, { portable: true }); - if (!key) { - continue; - } + for (const [key, value] of listNormalizedPortableEnvEntries(overrides)) { const upper = key.toUpperCase(); // PATH is part of the security boundary (command resolution + safe-bin checks). Never allow // request-scoped PATH overrides from agents/gateways. @@ -140,14 +146,7 @@ export function sanitizeSystemRunEnvOverrides(params?: { return overrides; } const filtered: Record = {}; - for (const [rawKey, value] of Object.entries(overrides)) { - if (typeof value !== "string") { - continue; - } - const key = normalizeEnvVarKey(rawKey, { portable: true }); - if (!key) { - continue; - } + for (const [key, value] of listNormalizedPortableEnvEntries(overrides)) { if (!HOST_SHELL_WRAPPER_ALLOWED_OVERRIDE_ENV_KEYS.has(key.toUpperCase())) { continue; } diff --git a/src/infra/http-body.ts b/src/infra/http-body.ts index cfa65560028..b578bf14552 100644 --- a/src/infra/http-body.ts +++ b/src/infra/http-body.ts @@ -84,6 +84,12 @@ type RequestBodyLimitValues = { timeoutMs: number; }; +type RequestBodyChunkProgress = { + buffer: Buffer; + totalBytes: number; + exceeded: boolean; +}; + function resolveRequestBodyLimitValues(options: { maxBytes: number; timeoutMs?: number; @@ -98,6 +104,20 @@ function resolveRequestBodyLimitValues(options: { return { maxBytes, timeoutMs }; } +function advanceRequestBodyChunk( + chunk: Buffer | string, + totalBytes: number, + maxBytes: number, +): RequestBodyChunkProgress { + const buffer = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk); + const nextTotalBytes = totalBytes + buffer.length; + return { + buffer, + totalBytes: nextTotalBytes, + exceeded: nextTotalBytes > maxBytes, + }; +} + export async function readRequestBodyWithLimit( req: IncomingMessage, options: ReadRequestBodyOptions, @@ -155,9 +175,9 @@ export async function readRequestBodyWithLimit( if (done) { return; } - const buffer = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk); - totalBytes += buffer.length; - if (totalBytes > maxBytes) { + const progress = advanceRequestBodyChunk(chunk, totalBytes, maxBytes); + totalBytes = progress.totalBytes; + if (progress.exceeded) { const error = new RequestBodyLimitError({ code: "PAYLOAD_TOO_LARGE" }); if (!req.destroyed) { req.destroy(); @@ -165,7 +185,7 @@ export async function readRequestBodyWithLimit( fail(error); return; } - chunks.push(buffer); + chunks.push(progress.buffer); }; const onEnd = () => { @@ -313,9 +333,9 @@ export function installRequestBodyLimitGuard( if (done) { return; } - const buffer = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk); - totalBytes += buffer.length; - if (totalBytes > maxBytes) { + const progress = advanceRequestBodyChunk(chunk, totalBytes, maxBytes); + totalBytes = progress.totalBytes; + if (progress.exceeded) { trip(new RequestBodyLimitError({ code: "PAYLOAD_TOO_LARGE" })); } }; diff --git a/src/infra/install-flow.test.ts b/src/infra/install-flow.test.ts index 1c3c46ac5ee..a1a7e80e3ad 100644 --- a/src/infra/install-flow.test.ts +++ b/src/infra/install-flow.test.ts @@ -6,6 +6,19 @@ import * as archive from "./archive.js"; import { resolveExistingInstallPath, withExtractedArchiveRoot } from "./install-flow.js"; import * as installSource from "./install-source-utils.js"; +async function runExtractedArchiveFailureCase(configureArchive: () => void) { + vi.spyOn(installSource, "withTempDir").mockImplementation( + async (_prefix, fn) => await fn("/tmp/openclaw-install-flow"), + ); + configureArchive(); + return await withExtractedArchiveRoot({ + archivePath: "/tmp/plugin.tgz", + tempDirPrefix: "openclaw-plugin-", + timeoutMs: 1000, + onExtracted: async () => ({ ok: true as const }), + }); +} + describe("resolveExistingInstallPath", () => { let fixtureRoot = ""; @@ -84,16 +97,8 @@ describe("withExtractedArchiveRoot", () => { }); it("returns extract failure when extraction throws", async () => { - vi.spyOn(installSource, "withTempDir").mockImplementation( - async (_prefix, fn) => await fn("/tmp/openclaw-install-flow"), - ); - vi.spyOn(archive, "extractArchive").mockRejectedValue(new Error("boom")); - - const result = await withExtractedArchiveRoot({ - archivePath: "/tmp/plugin.tgz", - tempDirPrefix: "openclaw-plugin-", - timeoutMs: 1000, - onExtracted: async () => ({ ok: true as const }), + const result = await runExtractedArchiveFailureCase(() => { + vi.spyOn(archive, "extractArchive").mockRejectedValue(new Error("boom")); }); expect(result).toEqual({ @@ -103,17 +108,9 @@ describe("withExtractedArchiveRoot", () => { }); it("returns root-resolution failure when archive layout is invalid", async () => { - vi.spyOn(installSource, "withTempDir").mockImplementation( - async (_prefix, fn) => await fn("/tmp/openclaw-install-flow"), - ); - vi.spyOn(archive, "extractArchive").mockResolvedValue(undefined); - vi.spyOn(archive, "resolvePackedRootDir").mockRejectedValue(new Error("invalid layout")); - - const result = await withExtractedArchiveRoot({ - archivePath: "/tmp/plugin.tgz", - tempDirPrefix: "openclaw-plugin-", - timeoutMs: 1000, - onExtracted: async () => ({ ok: true as const }), + const result = await runExtractedArchiveFailureCase(() => { + vi.spyOn(archive, "extractArchive").mockResolvedValue(undefined); + vi.spyOn(archive, "resolvePackedRootDir").mockRejectedValue(new Error("invalid layout")); }); expect(result).toEqual({ diff --git a/src/infra/openclaw-root.ts b/src/infra/openclaw-root.ts index 55b6bf7b91a..8015fcc8450 100644 --- a/src/infra/openclaw-root.ts +++ b/src/infra/openclaw-root.ts @@ -5,11 +5,14 @@ import { fileURLToPath } from "node:url"; const CORE_PACKAGE_NAMES = new Set(["openclaw"]); +function parsePackageName(raw: string): string | null { + const parsed = JSON.parse(raw) as { name?: unknown }; + return typeof parsed.name === "string" ? parsed.name : null; +} + async function readPackageName(dir: string): Promise { try { - const raw = await fs.readFile(path.join(dir, "package.json"), "utf-8"); - const parsed = JSON.parse(raw) as { name?: unknown }; - return typeof parsed.name === "string" ? parsed.name : null; + return parsePackageName(await fs.readFile(path.join(dir, "package.json"), "utf-8")); } catch { return null; } @@ -17,9 +20,7 @@ async function readPackageName(dir: string): Promise { function readPackageNameSync(dir: string): string | null { try { - const raw = fsSync.readFileSync(path.join(dir, "package.json"), "utf-8"); - const parsed = JSON.parse(raw) as { name?: unknown }; - return typeof parsed.name === "string" ? parsed.name : null; + return parsePackageName(fsSync.readFileSync(path.join(dir, "package.json"), "utf-8")); } catch { return null; } diff --git a/src/infra/plugin-install-path-warnings.test.ts b/src/infra/plugin-install-path-warnings.test.ts index eef3348fb06..eb7ba108df1 100644 --- a/src/infra/plugin-install-path-warnings.test.ts +++ b/src/infra/plugin-install-path-warnings.test.ts @@ -7,6 +7,25 @@ import { formatPluginInstallPathIssue, } from "./plugin-install-path-warnings.js"; +async function detectMatrixCustomPathIssue(sourcePath: string | ((pluginPath: string) => string)) { + return withTempHome(async (home) => { + const pluginPath = path.join(home, "matrix-plugin"); + await fs.mkdir(pluginPath, { recursive: true }); + const resolvedSourcePath = + typeof sourcePath === "function" ? sourcePath(pluginPath) : sourcePath; + const issue = await detectPluginInstallPathIssue({ + pluginId: "matrix", + install: { + source: "path", + sourcePath: resolvedSourcePath, + installPath: pluginPath, + }, + }); + + return { issue, pluginPath }; + }); +} + describe("plugin install path warnings", () => { it("ignores non-path installs and blank path candidates", async () => { expect( @@ -57,46 +76,22 @@ describe("plugin install path warnings", () => { }); it("uses the second candidate path when the first one is stale", async () => { - await withTempHome(async (home) => { - const pluginPath = path.join(home, "matrix-plugin"); - await fs.mkdir(pluginPath, { recursive: true }); - - const issue = await detectPluginInstallPathIssue({ - pluginId: "matrix", - install: { - source: "path", - sourcePath: "/tmp/openclaw-matrix-missing", - installPath: pluginPath, - }, - }); - - expect(issue).toEqual({ - kind: "custom-path", - pluginId: "matrix", - path: pluginPath, - }); + const { issue, pluginPath } = await detectMatrixCustomPathIssue("/tmp/openclaw-matrix-missing"); + expect(issue).toEqual({ + kind: "custom-path", + pluginId: "matrix", + path: pluginPath, }); }); it("detects active custom plugin install paths", async () => { - await withTempHome(async (home) => { - const pluginPath = path.join(home, "matrix-plugin"); - await fs.mkdir(pluginPath, { recursive: true }); - - const issue = await detectPluginInstallPathIssue({ - pluginId: "matrix", - install: { - source: "path", - sourcePath: pluginPath, - installPath: pluginPath, - }, - }); - - expect(issue).toEqual({ - kind: "custom-path", - pluginId: "matrix", - path: pluginPath, - }); + const { issue, pluginPath } = await detectMatrixCustomPathIssue( + (resolvedPluginPath) => resolvedPluginPath, + ); + expect(issue).toEqual({ + kind: "custom-path", + pluginId: "matrix", + path: pluginPath, }); }); diff --git a/src/infra/process-respawn.test.ts b/src/infra/process-respawn.test.ts index bacf4e1b24b..804dca82c22 100644 --- a/src/infra/process-respawn.test.ts +++ b/src/infra/process-respawn.test.ts @@ -53,7 +53,10 @@ function clearSupervisorHints() { } } -function expectLaunchdSupervisedWithoutKickstart(params?: { launchJobLabel?: string }) { +function expectLaunchdSupervisedWithoutKickstart(params?: { + launchJobLabel?: string; + detailContains?: string; +}) { setPlatform("darwin"); if (params?.launchJobLabel) { process.env.LAUNCH_JOB_LABEL = params.launchJobLabel; @@ -61,6 +64,9 @@ function expectLaunchdSupervisedWithoutKickstart(params?: { launchJobLabel?: str process.env.OPENCLAW_LAUNCHD_LABEL = "ai.openclaw.gateway"; const result = restartGatewayProcessWithFreshPid(); expect(result.mode).toBe("supervised"); + if (params?.detailContains) { + expect(result.detail).toContain(params.detailContains); + } expect(scheduleDetachedLaunchdRestartHandoffMock).toHaveBeenCalledWith({ env: process.env, mode: "start-after-exit", @@ -80,18 +86,10 @@ describe("restartGatewayProcessWithFreshPid", () => { it("returns supervised when launchd hints are present on macOS (no kickstart)", () => { clearSupervisorHints(); - setPlatform("darwin"); - process.env.LAUNCH_JOB_LABEL = "ai.openclaw.gateway"; - const result = restartGatewayProcessWithFreshPid(); - expect(result.mode).toBe("supervised"); - expect(result.detail).toContain("launchd restart handoff"); - expect(scheduleDetachedLaunchdRestartHandoffMock).toHaveBeenCalledWith({ - env: process.env, - mode: "start-after-exit", - waitForPid: process.pid, + expectLaunchdSupervisedWithoutKickstart({ + launchJobLabel: "ai.openclaw.gateway", + detailContains: "launchd restart handoff", }); - expect(triggerOpenClawRestartMock).not.toHaveBeenCalled(); - expect(spawnMock).not.toHaveBeenCalled(); }); it("returns supervised on macOS when launchd label is set (no kickstart)", () => { diff --git a/src/infra/push-apns.test.ts b/src/infra/push-apns.test.ts index 83da4ae3165..a2c616e81b4 100644 --- a/src/infra/push-apns.test.ts +++ b/src/infra/push-apns.test.ts @@ -49,6 +49,29 @@ async function makeTempDir(): Promise { return dir; } +function createDirectApnsSendFixture(params: { + nodeId: string; + environment: "sandbox" | "production"; + sendResult: { status: number; apnsId: string; body: string }; +}) { + return { + send: vi.fn().mockResolvedValue(params.sendResult), + registration: { + nodeId: params.nodeId, + transport: "direct" as const, + token: "ABCD1234ABCD1234ABCD1234ABCD1234", + topic: "ai.openclaw.ios", + environment: params.environment, + updatedAtMs: 1, + }, + auth: { + teamId: "TEAM123", + keyId: "KEY123", + privateKey: testAuthPrivateKey, + }, + }; +} + afterEach(async () => { vi.unstubAllGlobals(); while (tempDirs.length > 0) { @@ -447,29 +470,22 @@ describe("push APNs env config", () => { describe("push APNs send semantics", () => { it("sends alert pushes with alert headers and payload", async () => { - const send = vi.fn().mockResolvedValue({ - status: 200, - apnsId: "apns-alert-id", - body: "", + const { send, registration, auth } = createDirectApnsSendFixture({ + nodeId: "ios-node-alert", + environment: "sandbox", + sendResult: { + status: 200, + apnsId: "apns-alert-id", + body: "", + }, }); const result = await sendApnsAlert({ - registration: { - nodeId: "ios-node-alert", - transport: "direct", - token: "ABCD1234ABCD1234ABCD1234ABCD1234", - topic: "ai.openclaw.ios", - environment: "sandbox", - updatedAtMs: 1, - }, + registration, nodeId: "ios-node-alert", title: "Wake", body: "Ping", - auth: { - teamId: "TEAM123", - keyId: "KEY123", - privateKey: testAuthPrivateKey, - }, + auth, requestSender: send, }); @@ -493,28 +509,21 @@ describe("push APNs send semantics", () => { }); it("sends background wake pushes with silent payload semantics", async () => { - const send = vi.fn().mockResolvedValue({ - status: 200, - apnsId: "apns-wake-id", - body: "", + const { send, registration, auth } = createDirectApnsSendFixture({ + nodeId: "ios-node-wake", + environment: "production", + sendResult: { + status: 200, + apnsId: "apns-wake-id", + body: "", + }, }); const result = await sendApnsBackgroundWake({ - registration: { - nodeId: "ios-node-wake", - transport: "direct", - token: "ABCD1234ABCD1234ABCD1234ABCD1234", - topic: "ai.openclaw.ios", - environment: "production", - updatedAtMs: 1, - }, + registration, nodeId: "ios-node-wake", wakeReason: "node.invoke", - auth: { - teamId: "TEAM123", - keyId: "KEY123", - privateKey: testAuthPrivateKey, - }, + auth, requestSender: send, }); @@ -542,27 +551,20 @@ describe("push APNs send semantics", () => { }); it("defaults background wake reason when not provided", async () => { - const send = vi.fn().mockResolvedValue({ - status: 200, - apnsId: "apns-wake-default-reason-id", - body: "", + const { send, registration, auth } = createDirectApnsSendFixture({ + nodeId: "ios-node-wake-default-reason", + environment: "sandbox", + sendResult: { + status: 200, + apnsId: "apns-wake-default-reason-id", + body: "", + }, }); await sendApnsBackgroundWake({ - registration: { - nodeId: "ios-node-wake-default-reason", - transport: "direct", - token: "ABCD1234ABCD1234ABCD1234ABCD1234", - topic: "ai.openclaw.ios", - environment: "sandbox", - updatedAtMs: 1, - }, + registration, nodeId: "ios-node-wake-default-reason", - auth: { - teamId: "TEAM123", - keyId: "KEY123", - privateKey: testAuthPrivateKey, - }, + auth, requestSender: send, }); diff --git a/src/infra/update-runner.test.ts b/src/infra/update-runner.test.ts index 0ba8e1ce3f9..bb9be0d5be7 100644 --- a/src/infra/update-runner.test.ts +++ b/src/infra/update-runner.test.ts @@ -187,6 +187,21 @@ describe("runGatewayUpdate", () => { ); } + async function writeGlobalPackageVersion(pkgRoot: string, version = "2.0.0") { + await fs.writeFile( + path.join(pkgRoot, "package.json"), + JSON.stringify({ name: "openclaw", version }), + "utf-8", + ); + } + + async function createGlobalPackageFixture(rootDir: string) { + const nodeModules = path.join(rootDir, "node_modules"); + const pkgRoot = path.join(nodeModules, "openclaw"); + await seedGlobalPackageRoot(pkgRoot); + return { nodeModules, pkgRoot }; + } + function createGlobalNpmUpdateRunner(params: { pkgRoot: string; nodeModules: string; @@ -366,13 +381,17 @@ describe("runGatewayUpdate", () => { pkgRoot: string; npmRootOutput?: string; installCommand: string; - onInstall?: () => Promise; + gitRootMode?: "not-git" | "missing"; + onInstall?: (options?: { env?: NodeJS.ProcessEnv }) => Promise; }) => { const calls: string[] = []; - const runCommand = async (argv: string[]) => { + const runCommand = async (argv: string[], options?: { env?: NodeJS.ProcessEnv }) => { const key = argv.join(" "); calls.push(key); if (key === `git -C ${params.pkgRoot} rev-parse --show-toplevel`) { + if (params.gitRootMode === "missing") { + throw Object.assign(new Error("spawn git ENOENT"), { code: "ENOENT" }); + } return { stdout: "", stderr: "not a git repository", code: 128 }; } if (key === "npm root -g") { @@ -385,7 +404,7 @@ describe("runGatewayUpdate", () => { return { stdout: "", stderr: "", code: 1 }; } if (key === params.installCommand) { - await params.onInstall?.(); + await params.onInstall?.(options); return { stdout: "ok", stderr: "", code: 0 }; } return { stdout: "", stderr: "", code: 0 }; @@ -423,32 +442,14 @@ describe("runGatewayUpdate", () => { }); it("falls back to global npm update when git is missing from PATH", async () => { - const nodeModules = path.join(tempDir, "node_modules"); - const pkgRoot = path.join(nodeModules, "openclaw"); - await seedGlobalPackageRoot(pkgRoot); - - const calls: string[] = []; - const runCommand = async (argv: string[]): Promise => { - const key = argv.join(" "); - calls.push(key); - if (key === `git -C ${pkgRoot} rev-parse --show-toplevel`) { - throw Object.assign(new Error("spawn git ENOENT"), { code: "ENOENT" }); - } - if (key === "npm root -g") { - return { stdout: nodeModules, stderr: "", code: 0 }; - } - if (key === "pnpm root -g") { - return { stdout: "", stderr: "", code: 1 }; - } - if (key === "npm i -g openclaw@latest --no-fund --no-audit --loglevel=error") { - await fs.writeFile( - path.join(pkgRoot, "package.json"), - JSON.stringify({ name: "openclaw", version: "2.0.0" }), - "utf-8", - ); - } - return { stdout: "ok", stderr: "", code: 0 }; - }; + const { nodeModules, pkgRoot } = await createGlobalPackageFixture(tempDir); + const { calls, runCommand } = createGlobalInstallHarness({ + pkgRoot, + npmRootOutput: nodeModules, + installCommand: "npm i -g openclaw@latest --no-fund --no-audit --loglevel=error", + gitRootMode: "missing", + onInstall: async () => writeGlobalPackageVersion(pkgRoot), + }); const result = await runWithCommand(runCommand, { cwd: pkgRoot }); @@ -537,35 +538,17 @@ describe("runGatewayUpdate", () => { await fs.mkdir(portableGitMingw, { recursive: true }); await fs.mkdir(portableGitUsr, { recursive: true }); - const nodeModules = path.join(tempDir, "node_modules"); - const pkgRoot = path.join(nodeModules, "openclaw"); - await seedGlobalPackageRoot(pkgRoot); - let installEnv: NodeJS.ProcessEnv | undefined; - const runCommand = async ( - argv: string[], - options?: { env?: NodeJS.ProcessEnv }, - ): Promise => { - const key = argv.join(" "); - if (key === `git -C ${pkgRoot} rev-parse --show-toplevel`) { - return { stdout: "", stderr: "not a git repository", code: 128 }; - } - if (key === "npm root -g") { - return { stdout: nodeModules, stderr: "", code: 0 }; - } - if (key === "pnpm root -g") { - return { stdout: "", stderr: "", code: 1 }; - } - if (key === "npm i -g openclaw@latest --no-fund --no-audit --loglevel=error") { + const { nodeModules, pkgRoot } = await createGlobalPackageFixture(tempDir); + const { runCommand } = createGlobalInstallHarness({ + pkgRoot, + npmRootOutput: nodeModules, + installCommand: "npm i -g openclaw@latest --no-fund --no-audit --loglevel=error", + onInstall: async (options) => { installEnv = options?.env; - await fs.writeFile( - path.join(pkgRoot, "package.json"), - JSON.stringify({ name: "openclaw", version: "2.0.0" }), - "utf-8", - ); - } - return { stdout: "ok", stderr: "", code: 0 }; - }; + await writeGlobalPackageVersion(pkgRoot); + }, + }); await withEnvAsync({ LOCALAPPDATA: localAppData }, async () => { const result = await runWithCommand(runCommand, { cwd: pkgRoot }); @@ -584,35 +567,15 @@ describe("runGatewayUpdate", () => { }); it("uses OPENCLAW_UPDATE_PACKAGE_SPEC for global package updates", async () => { - const nodeModules = path.join(tempDir, "node_modules"); - const pkgRoot = path.join(nodeModules, "openclaw"); - await seedGlobalPackageRoot(pkgRoot); - - const calls: string[] = []; - const runCommand = async (argv: string[]): Promise => { - const key = argv.join(" "); - calls.push(key); - if (key === `git -C ${pkgRoot} rev-parse --show-toplevel`) { - return { stdout: "", stderr: "not a git repository", code: 128 }; - } - if (key === "npm root -g") { - return { stdout: nodeModules, stderr: "", code: 0 }; - } - if (key === "pnpm root -g") { - return { stdout: "", stderr: "", code: 1 }; - } - if ( - key === - "npm i -g http://10.211.55.2:8138/openclaw-next.tgz --no-fund --no-audit --loglevel=error" - ) { - await fs.writeFile( - path.join(pkgRoot, "package.json"), - JSON.stringify({ name: "openclaw", version: "2.0.0" }), - "utf-8", - ); - } - return { stdout: "ok", stderr: "", code: 0 }; - }; + const { nodeModules, pkgRoot } = await createGlobalPackageFixture(tempDir); + const expectedInstallCommand = + "npm i -g http://10.211.55.2:8138/openclaw-next.tgz --no-fund --no-audit --loglevel=error"; + const { calls, runCommand } = createGlobalInstallHarness({ + pkgRoot, + npmRootOutput: nodeModules, + installCommand: expectedInstallCommand, + onInstall: async () => writeGlobalPackageVersion(pkgRoot), + }); await withEnvAsync( { OPENCLAW_UPDATE_PACKAGE_SPEC: "http://10.211.55.2:8138/openclaw-next.tgz" }, @@ -622,27 +585,21 @@ describe("runGatewayUpdate", () => { }, ); - expect(calls).toContain( - "npm i -g http://10.211.55.2:8138/openclaw-next.tgz --no-fund --no-audit --loglevel=error", - ); + expect(calls).toContain(expectedInstallCommand); }); it("updates global bun installs when detected", async () => { const bunInstall = path.join(tempDir, "bun-install"); await withEnvAsync({ BUN_INSTALL: bunInstall }, async () => { - const bunGlobalRoot = path.join(bunInstall, "install", "global", "node_modules"); - const pkgRoot = path.join(bunGlobalRoot, "openclaw"); - await seedGlobalPackageRoot(pkgRoot); + const { pkgRoot } = await createGlobalPackageFixture( + path.join(bunInstall, "install", "global"), + ); const { calls, runCommand } = createGlobalInstallHarness({ pkgRoot, installCommand: "bun add -g openclaw@latest", onInstall: async () => { - await fs.writeFile( - path.join(pkgRoot, "package.json"), - JSON.stringify({ name: "openclaw", version: "2.0.0" }), - "utf-8", - ); + await writeGlobalPackageVersion(pkgRoot); }, }); diff --git a/src/memory/search-manager.test.ts b/src/memory/search-manager.test.ts index 1f705aeddcf..a4feba3f25b 100644 --- a/src/memory/search-manager.test.ts +++ b/src/memory/search-manager.test.ts @@ -29,48 +29,67 @@ function createManagerStatus(params: { }; } +function createManagerMock(params: { + backend: "qmd" | "builtin"; + provider: string; + model: string; + requestedProvider: string; + searchResults?: Array<{ + path: string; + startLine: number; + endLine: number; + score: number; + snippet: string; + source: "memory"; + }>; + withMemorySourceCounts?: boolean; +}) { + return { + search: vi.fn(async () => params.searchResults ?? []), + readFile: vi.fn(async () => ({ text: "", path: "MEMORY.md" })), + status: vi.fn(() => + createManagerStatus({ + backend: params.backend, + provider: params.provider, + model: params.model, + requestedProvider: params.requestedProvider, + withMemorySourceCounts: params.withMemorySourceCounts, + }), + ), + sync: vi.fn(async () => {}), + probeEmbeddingAvailability: vi.fn(async () => ({ ok: true })), + probeVectorAvailability: vi.fn(async () => true), + close: vi.fn(async () => {}), + }; +} + const mockPrimary = vi.hoisted(() => ({ - search: vi.fn(async () => []), - readFile: vi.fn(async () => ({ text: "", path: "MEMORY.md" })), - status: vi.fn(() => - createManagerStatus({ - backend: "qmd", - provider: "qmd", - model: "qmd", - requestedProvider: "qmd", - withMemorySourceCounts: true, - }), - ), - sync: vi.fn(async () => {}), - probeEmbeddingAvailability: vi.fn(async () => ({ ok: true })), - probeVectorAvailability: vi.fn(async () => true), - close: vi.fn(async () => {}), + ...createManagerMock({ + backend: "qmd", + provider: "qmd", + model: "qmd", + requestedProvider: "qmd", + withMemorySourceCounts: true, + }), })); const fallbackManager = vi.hoisted(() => ({ - search: vi.fn(async () => [ - { - path: "MEMORY.md", - startLine: 1, - endLine: 1, - score: 1, - snippet: "fallback", - source: "memory" as const, - }, - ]), - readFile: vi.fn(async () => ({ text: "", path: "MEMORY.md" })), - status: vi.fn(() => - createManagerStatus({ - backend: "builtin", - provider: "openai", - model: "text-embedding-3-small", - requestedProvider: "openai", - }), - ), - sync: vi.fn(async () => {}), - probeEmbeddingAvailability: vi.fn(async () => ({ ok: true })), - probeVectorAvailability: vi.fn(async () => true), - close: vi.fn(async () => {}), + ...createManagerMock({ + backend: "builtin", + provider: "openai", + model: "text-embedding-3-small", + requestedProvider: "openai", + searchResults: [ + { + path: "MEMORY.md", + startLine: 1, + endLine: 1, + score: 1, + snippet: "fallback", + source: "memory", + }, + ], + }), })); const fallbackSearch = fallbackManager.search; diff --git a/src/memory/temporal-decay.test.ts b/src/memory/temporal-decay.test.ts index 1c01c16ea35..edb4df553d6 100644 --- a/src/memory/temporal-decay.test.ts +++ b/src/memory/temporal-decay.test.ts @@ -20,6 +20,37 @@ async function makeTempDir(): Promise { return dir; } +function createVectorMemoryEntry(params: { + id: string; + path: string; + snippet: string; + vectorScore: number; +}) { + return { + id: params.id, + path: params.path, + startLine: 1, + endLine: 1, + source: "memory" as const, + snippet: params.snippet, + vectorScore: params.vectorScore, + }; +} + +async function mergeVectorResultsWithTemporalDecay( + vector: Parameters[0]["vector"], +) { + return mergeHybridResults({ + vectorWeight: 1, + textWeight: 0, + temporalDecay: { enabled: true, halfLifeDays: 30 }, + mmr: { enabled: false }, + nowMs: NOW_MS, + vector, + keyword: [], + }); +} + afterEach(async () => { await Promise.all( tempDirs.splice(0).map(async (dir) => { @@ -75,77 +106,46 @@ describe("temporal decay", () => { }); it("applies decay in hybrid merging before ranking", async () => { - const merged = await mergeHybridResults({ - vectorWeight: 1, - textWeight: 0, - temporalDecay: { enabled: true, halfLifeDays: 30 }, - mmr: { enabled: false }, - nowMs: NOW_MS, - vector: [ - { - id: "old", - path: "memory/2025-01-01.md", - startLine: 1, - endLine: 1, - source: "memory", - snippet: "old but high", - vectorScore: 0.95, - }, - { - id: "new", - path: "memory/2026-02-10.md", - startLine: 1, - endLine: 1, - source: "memory", - snippet: "new and relevant", - vectorScore: 0.8, - }, - ], - keyword: [], - }); + const merged = await mergeVectorResultsWithTemporalDecay([ + createVectorMemoryEntry({ + id: "old", + path: "memory/2025-01-01.md", + snippet: "old but high", + vectorScore: 0.95, + }), + createVectorMemoryEntry({ + id: "new", + path: "memory/2026-02-10.md", + snippet: "new and relevant", + vectorScore: 0.8, + }), + ]); expect(merged[0]?.path).toBe("memory/2026-02-10.md"); expect(merged[0]?.score ?? 0).toBeGreaterThan(merged[1]?.score ?? 0); }); it("handles future dates, zero age, and very old memories", async () => { - const merged = await mergeHybridResults({ - vectorWeight: 1, - textWeight: 0, - temporalDecay: { enabled: true, halfLifeDays: 30 }, - mmr: { enabled: false }, - nowMs: NOW_MS, - vector: [ - { - id: "future", - path: "memory/2099-01-01.md", - startLine: 1, - endLine: 1, - source: "memory", - snippet: "future", - vectorScore: 0.9, - }, - { - id: "today", - path: "memory/2026-02-10.md", - startLine: 1, - endLine: 1, - source: "memory", - snippet: "today", - vectorScore: 0.8, - }, - { - id: "very-old", - path: "memory/2000-01-01.md", - startLine: 1, - endLine: 1, - source: "memory", - snippet: "ancient", - vectorScore: 1, - }, - ], - keyword: [], - }); + const merged = await mergeVectorResultsWithTemporalDecay([ + createVectorMemoryEntry({ + id: "future", + path: "memory/2099-01-01.md", + snippet: "future", + vectorScore: 0.9, + }), + createVectorMemoryEntry({ + id: "today", + path: "memory/2026-02-10.md", + snippet: "today", + vectorScore: 0.8, + }), + createVectorMemoryEntry({ + id: "very-old", + path: "memory/2000-01-01.md", + snippet: "ancient", + vectorScore: 1, + }), + ]); const byPath = new Map(merged.map((entry) => [entry.path, entry])); expect(byPath.get("memory/2099-01-01.md")?.score).toBeCloseTo(0.9); diff --git a/src/node-host/invoke-system-run.ts b/src/node-host/invoke-system-run.ts index 3730e3b2824..32bd2d6ff79 100644 --- a/src/node-host/invoke-system-run.ts +++ b/src/node-host/invoke-system-run.ts @@ -182,6 +182,25 @@ async function sendSystemRunDenied( }); } +async function sendSystemRunCompleted( + opts: Pick, + execution: SystemRunExecutionContext, + result: ExecFinishedResult, + payloadJSON: string, +) { + await opts.sendExecFinishedEvent({ + sessionKey: execution.sessionKey, + runId: execution.runId, + commandText: execution.commandText, + result, + suppressNotifyOnExit: execution.suppressNotifyOnExit, + }); + await opts.sendInvokeResult({ + ok: true, + payloadJSON, + }); +} + export { formatSystemRunAllowlistMissMessage } from "./exec-policy.js"; export { buildSystemRunApprovalPlan } from "./invoke-system-run-plan.js"; @@ -462,17 +481,7 @@ async function executeSystemRunPhase( return; } else { const result: ExecHostRunResult = response.payload; - await opts.sendExecFinishedEvent({ - sessionKey: phase.sessionKey, - runId: phase.runId, - commandText: phase.commandText, - result, - suppressNotifyOnExit: phase.suppressNotifyOnExit, - }); - await opts.sendInvokeResult({ - ok: true, - payloadJSON: JSON.stringify(result), - }); + await sendSystemRunCompleted(opts, phase.execution, result, JSON.stringify(result)); return; } } @@ -530,17 +539,11 @@ async function executeSystemRunPhase( const result = await opts.runCommand(execArgv, phase.cwd, phase.env, phase.timeoutMs); applyOutputTruncation(result); - await opts.sendExecFinishedEvent({ - sessionKey: phase.sessionKey, - runId: phase.runId, - commandText: phase.commandText, + await sendSystemRunCompleted( + opts, + phase.execution, result, - suppressNotifyOnExit: phase.suppressNotifyOnExit, - }); - - await opts.sendInvokeResult({ - ok: true, - payloadJSON: JSON.stringify({ + JSON.stringify({ exitCode: result.exitCode, timedOut: result.timedOut, success: result.success, @@ -548,7 +551,7 @@ async function executeSystemRunPhase( stderr: result.stderr, error: result.error ?? null, }), - }); + ); } export async function handleSystemRunInvoke(opts: HandleSystemRunInvokeOptions): Promise { diff --git a/src/node-host/runner.credentials.test.ts b/src/node-host/runner.credentials.test.ts index 6138a6b954e..c7a04951130 100644 --- a/src/node-host/runner.credentials.test.ts +++ b/src/node-host/runner.credentials.test.ts @@ -19,6 +19,17 @@ function createRemoteGatewayTokenRefConfig(tokenId: string): OpenClawConfig { } as OpenClawConfig; } +async function expectNoGatewayCredentials( + config: OpenClawConfig, + env: Record, +) { + await withEnvAsync(env, async () => { + const credentials = await resolveNodeHostGatewayCredentials({ config }); + expect(credentials.token).toBeUndefined(); + expect(credentials.password).toBeUndefined(); + }); +} + describe("resolveNodeHostGatewayCredentials", () => { it("does not inherit gateway.remote token in local mode", async () => { const config = { @@ -28,17 +39,10 @@ describe("resolveNodeHostGatewayCredentials", () => { }, } as OpenClawConfig; - await withEnvAsync( - { - OPENCLAW_GATEWAY_TOKEN: undefined, - OPENCLAW_GATEWAY_PASSWORD: undefined, - }, - async () => { - const credentials = await resolveNodeHostGatewayCredentials({ config }); - expect(credentials.token).toBeUndefined(); - expect(credentials.password).toBeUndefined(); - }, - ); + await expectNoGatewayCredentials(config, { + OPENCLAW_GATEWAY_TOKEN: undefined, + OPENCLAW_GATEWAY_PASSWORD: undefined, + }); }); it("ignores unresolved gateway.remote token refs in local mode", async () => { @@ -56,18 +60,11 @@ describe("resolveNodeHostGatewayCredentials", () => { }, } as OpenClawConfig; - await withEnvAsync( - { - OPENCLAW_GATEWAY_TOKEN: undefined, - OPENCLAW_GATEWAY_PASSWORD: undefined, - MISSING_REMOTE_GATEWAY_TOKEN: undefined, - }, - async () => { - const credentials = await resolveNodeHostGatewayCredentials({ config }); - expect(credentials.token).toBeUndefined(); - expect(credentials.password).toBeUndefined(); - }, - ); + await expectNoGatewayCredentials(config, { + OPENCLAW_GATEWAY_TOKEN: undefined, + OPENCLAW_GATEWAY_PASSWORD: undefined, + MISSING_REMOTE_GATEWAY_TOKEN: undefined, + }); }); it("resolves remote token SecretRef values", async () => { diff --git a/src/param-key.ts b/src/param-key.ts new file mode 100644 index 00000000000..9b86eccfc1b --- /dev/null +++ b/src/param-key.ts @@ -0,0 +1,17 @@ +export function toSnakeCaseKey(key: string): string { + return key + .replace(/([A-Z]+)([A-Z][a-z])/g, "$1_$2") + .replace(/([a-z0-9])([A-Z])/g, "$1_$2") + .toLowerCase(); +} + +export function readSnakeCaseParamRaw(params: Record, key: string): unknown { + if (Object.hasOwn(params, key)) { + return params[key]; + } + const snakeKey = toSnakeCaseKey(key); + if (snakeKey !== key && Object.hasOwn(params, snakeKey)) { + return params[snakeKey]; + } + return undefined; +} diff --git a/src/poll-params.ts b/src/poll-params.ts index 88dc6336d32..f6fc5546548 100644 --- a/src/poll-params.ts +++ b/src/poll-params.ts @@ -1,3 +1,5 @@ +import { readSnakeCaseParamRaw } from "./param-key.js"; + export type PollCreationParamKind = "string" | "stringArray" | "number" | "boolean"; export type PollCreationParamDef = { @@ -19,22 +21,8 @@ export type PollCreationParamName = keyof typeof POLL_CREATION_PARAM_DEFS; export const POLL_CREATION_PARAM_NAMES = Object.keys(POLL_CREATION_PARAM_DEFS); -function toSnakeCaseKey(key: string): string { - return key - .replace(/([A-Z]+)([A-Z][a-z])/g, "$1_$2") - .replace(/([a-z0-9])([A-Z])/g, "$1_$2") - .toLowerCase(); -} - function readPollParamRaw(params: Record, key: string): unknown { - if (Object.hasOwn(params, key)) { - return params[key]; - } - const snakeKey = toSnakeCaseKey(key); - if (snakeKey !== key && Object.hasOwn(params, snakeKey)) { - return params[snakeKey]; - } - return undefined; + return readSnakeCaseParamRaw(params, key); } export function resolveTelegramPollVisibility(params: { diff --git a/src/shared/assistant-identity-values.test.ts b/src/shared/assistant-identity-values.test.ts index 59792dc7fd9..f0e594cc7e7 100644 --- a/src/shared/assistant-identity-values.test.ts +++ b/src/shared/assistant-identity-values.test.ts @@ -5,6 +5,7 @@ describe("shared/assistant-identity-values", () => { it("returns undefined for missing or blank values", () => { expect(coerceIdentityValue(undefined, 10)).toBeUndefined(); expect(coerceIdentityValue(" ", 10)).toBeUndefined(); + expect(coerceIdentityValue(42 as unknown as string, 10)).toBeUndefined(); }); it("trims values and preserves strings within the limit", () => { @@ -14,4 +15,8 @@ describe("shared/assistant-identity-values", () => { it("truncates overlong trimmed values at the exact limit", () => { expect(coerceIdentityValue(" OpenClaw Assistant ", 8)).toBe("OpenClaw"); }); + + it("returns an empty string when truncating to a zero-length limit", () => { + expect(coerceIdentityValue(" OpenClaw ", 0)).toBe(""); + }); }); diff --git a/src/shared/chat-content.test.ts b/src/shared/chat-content.test.ts index d66ec203c72..0131865cef8 100644 --- a/src/shared/chat-content.test.ts +++ b/src/shared/chat-content.test.ts @@ -12,6 +12,7 @@ describe("shared/chat-content", () => { { type: "text", text: " hello " }, { type: "image_url", image_url: "https://example.com" }, { type: "text", text: "world" }, + { text: "ignored without type" }, null, ]), ).toBe("hello world"); @@ -37,6 +38,18 @@ describe("shared/chat-content", () => { }, ), ).toBe("hello\nworld"); + + expect( + extractTextFromChatContent( + [ + { type: "text", text: "keep" }, + { type: "text", text: "drop" }, + ], + { + sanitizeText: (text) => (text === "drop" ? " " : text), + }, + ), + ).toBe("keep"); }); it("returns null for unsupported or empty content", () => { diff --git a/src/shared/chat-envelope.test.ts b/src/shared/chat-envelope.test.ts index d04bc014e44..0bd513c1b61 100644 --- a/src/shared/chat-envelope.test.ts +++ b/src/shared/chat-envelope.test.ts @@ -4,18 +4,23 @@ import { stripEnvelope, stripMessageIdHints } from "./chat-envelope.js"; describe("shared/chat-envelope", () => { it("strips recognized channel and timestamp envelope prefixes only", () => { expect(stripEnvelope("[WhatsApp 2026-01-24 13:36] hello")).toBe("hello"); + expect(stripEnvelope("[Google Chat room] hello")).toBe("hello"); expect(stripEnvelope("[2026-01-24T13:36Z] hello")).toBe("hello"); + expect(stripEnvelope("[2026-01-24 13:36] hello")).toBe("hello"); expect(stripEnvelope("[Custom Sender] hello")).toBe("[Custom Sender] hello"); }); it("keeps non-envelope headers and preserves unmatched text", () => { expect(stripEnvelope("hello")).toBe("hello"); expect(stripEnvelope("[note] hello")).toBe("[note] hello"); + expect(stripEnvelope("[2026/01/24 13:36] hello")).toBe("[2026/01/24 13:36] hello"); }); it("removes standalone message id hint lines but keeps inline mentions", () => { expect(stripMessageIdHints("hello\n[message_id: abc123]")).toBe("hello"); expect(stripMessageIdHints("hello\n [message_id: abc123] \nworld")).toBe("hello\nworld"); + expect(stripMessageIdHints("[message_id: abc123]\nhello")).toBe("hello"); + expect(stripMessageIdHints("[message_id: abc123]")).toBe(""); expect(stripMessageIdHints("I typed [message_id: abc123] inline")).toBe( "I typed [message_id: abc123] inline", ); diff --git a/src/shared/chat-message-content.test.ts b/src/shared/chat-message-content.test.ts index db2d533cebd..50e41f82642 100644 --- a/src/shared/chat-message-content.test.ts +++ b/src/shared/chat-message-content.test.ts @@ -10,10 +10,19 @@ describe("shared/chat-message-content", () => { ).toBe("hello"); }); + it("preserves empty-string text in the first block", () => { + expect( + extractFirstTextBlock({ + content: [{ text: "" }, { text: "later" }], + }), + ).toBe(""); + }); + it("returns undefined for missing, empty, or non-text content", () => { expect(extractFirstTextBlock(null)).toBeUndefined(); expect(extractFirstTextBlock({ content: [] })).toBeUndefined(); expect(extractFirstTextBlock({ content: [{ type: "image" }] })).toBeUndefined(); expect(extractFirstTextBlock({ content: ["hello"] })).toBeUndefined(); + expect(extractFirstTextBlock({ content: [{ text: 1 }, { text: "later" }] })).toBeUndefined(); }); }); diff --git a/src/shared/device-auth.test.ts b/src/shared/device-auth.test.ts index 4675f866e54..a3bc6fa3956 100644 --- a/src/shared/device-auth.test.ts +++ b/src/shared/device-auth.test.ts @@ -5,6 +5,7 @@ describe("shared/device-auth", () => { it("trims device auth roles without further rewriting", () => { expect(normalizeDeviceAuthRole(" operator ")).toBe("operator"); expect(normalizeDeviceAuthRole("")).toBe(""); + expect(normalizeDeviceAuthRole(" NODE.Admin ")).toBe("NODE.Admin"); }); it("dedupes, trims, sorts, and filters auth scopes", () => { @@ -12,5 +13,11 @@ describe("shared/device-auth", () => { normalizeDeviceAuthScopes([" node.invoke ", "operator.read", "", "node.invoke", "a.scope"]), ).toEqual(["a.scope", "node.invoke", "operator.read"]); expect(normalizeDeviceAuthScopes(undefined)).toEqual([]); + expect(normalizeDeviceAuthScopes([" ", "\t", "\n"])).toEqual([]); + expect(normalizeDeviceAuthScopes(["z.scope", "A.scope", "m.scope"])).toEqual([ + "A.scope", + "m.scope", + "z.scope", + ]); }); }); diff --git a/src/shared/entry-metadata.test.ts b/src/shared/entry-metadata.test.ts index 64afb728a14..cf94453a62e 100644 --- a/src/shared/entry-metadata.test.ts +++ b/src/shared/entry-metadata.test.ts @@ -14,6 +14,15 @@ describe("shared/entry-metadata", () => { }); }); + it("keeps metadata precedence even when metadata values are blank", () => { + expect( + resolveEmojiAndHomepage({ + metadata: { emoji: "", homepage: " " }, + frontmatter: { emoji: "🙂", homepage: "https://example.com" }, + }), + ).toEqual({}); + }); + it("falls back through frontmatter homepage aliases and drops blanks", () => { expect( resolveEmojiAndHomepage({ @@ -29,5 +38,12 @@ describe("shared/entry-metadata", () => { frontmatter: { url: " " }, }), ).toEqual({}); + expect( + resolveEmojiAndHomepage({ + frontmatter: { url: " https://openclaw.ai/install " }, + }), + ).toEqual({ + homepage: "https://openclaw.ai/install", + }); }); }); diff --git a/src/shared/global-singleton.test.ts b/src/shared/global-singleton.test.ts index 3d537f5cc4b..34b4e8817f3 100644 --- a/src/shared/global-singleton.test.ts +++ b/src/shared/global-singleton.test.ts @@ -52,4 +52,14 @@ describe("resolveGlobalMap", () => { expect(resolveGlobalMap(TEST_MAP_KEY).get("a")).toBe(1); }); + + it("reuses a prepopulated global map without creating a new one", () => { + const existing = new Map([["a", 1]]); + (globalThis as Record)[TEST_MAP_KEY] = existing; + + const resolved = resolveGlobalMap(TEST_MAP_KEY); + + expect(resolved).toBe(existing); + expect(resolved.get("a")).toBe(1); + }); }); diff --git a/src/shared/model-param-b.test.ts b/src/shared/model-param-b.test.ts index 21c3dce79c8..7fb9a7b82d4 100644 --- a/src/shared/model-param-b.test.ts +++ b/src/shared/model-param-b.test.ts @@ -5,10 +5,13 @@ describe("shared/model-param-b", () => { it("extracts the largest valid b-sized parameter token", () => { expect(inferParamBFromIdOrName("llama-8b mixtral-22b")).toBe(22); expect(inferParamBFromIdOrName("Qwen 0.5B Instruct")).toBe(0.5); + expect(inferParamBFromIdOrName("prefix M7B and q4_32b")).toBe(32); }); it("ignores malformed, zero, and non-delimited matches", () => { expect(inferParamBFromIdOrName("abc70beta 0b x70b2")).toBeNull(); expect(inferParamBFromIdOrName("model 0b")).toBeNull(); + expect(inferParamBFromIdOrName("model b5")).toBeNull(); + expect(inferParamBFromIdOrName("foo70bbar")).toBeNull(); }); }); diff --git a/src/shared/net/ipv4.test.ts b/src/shared/net/ipv4.test.ts index a6d7ab2f84e..21ff99b982b 100644 --- a/src/shared/net/ipv4.test.ts +++ b/src/shared/net/ipv4.test.ts @@ -7,13 +7,18 @@ describe("shared/net/ipv4", () => { "IP address is required for custom bind mode", ); expect(validateDottedDecimalIPv4Input("")).toBe("IP address is required for custom bind mode"); + expect(validateDottedDecimalIPv4Input(" ")).toBe( + "Invalid IPv4 address (e.g., 192.168.1.100)", + ); }); it("accepts canonical dotted-decimal ipv4 only", () => { expect(validateDottedDecimalIPv4Input("192.168.1.100")).toBeUndefined(); + expect(validateDottedDecimalIPv4Input(" 192.168.1.100 ")).toBeUndefined(); expect(validateDottedDecimalIPv4Input("0177.0.0.1")).toBe( "Invalid IPv4 address (e.g., 192.168.1.100)", ); + expect(validateDottedDecimalIPv4Input("[192.168.1.100]")).toBeUndefined(); expect(validateDottedDecimalIPv4Input("example.com")).toBe( "Invalid IPv4 address (e.g., 192.168.1.100)", ); diff --git a/src/shared/node-resolve.test.ts b/src/shared/node-resolve.test.ts index 4af0c5a8a9b..2020073a910 100644 --- a/src/shared/node-resolve.test.ts +++ b/src/shared/node-resolve.test.ts @@ -20,6 +20,18 @@ describe("shared/node-resolve", () => { ).toBe("mac-123"); }); + it("passes the original node list to the default picker", () => { + expect( + resolveNodeIdFromNodeList(nodes, "", { + allowDefault: true, + pickDefaultNode: (entries) => { + expect(entries).toBe(nodes); + return entries[1] ?? null; + }, + }), + ).toBe("pi-456"); + }); + it("still throws when default selection is disabled or returns null", () => { expect(() => resolveNodeIdFromNodeList(nodes, " ")).toThrow(/node required/); expect(() => diff --git a/src/shared/process-scoped-map.test.ts b/src/shared/process-scoped-map.test.ts index 7389770643c..d4a0ff17a59 100644 --- a/src/shared/process-scoped-map.test.ts +++ b/src/shared/process-scoped-map.test.ts @@ -26,4 +26,14 @@ describe("shared/process-scoped-map", () => { expect(second).not.toBe(first); }); + + it("reuses a prepopulated process map without replacing it", () => { + const existing = new Map([["a", 1]]); + (process as unknown as Record)[MAP_KEY] = existing; + + const resolved = resolveProcessScopedMap(MAP_KEY); + + expect(resolved).toBe(existing); + expect(resolved.get("a")).toBe(1); + }); }); diff --git a/src/shared/string-sample.test.ts b/src/shared/string-sample.test.ts index 7ced1e7407a..f86e8e1b349 100644 --- a/src/shared/string-sample.test.ts +++ b/src/shared/string-sample.test.ts @@ -42,4 +42,14 @@ describe("summarizeStringEntries", () => { }), ).toBe("a, b, c, d, e, f (+1)"); }); + + it("does not add a suffix when the limit exactly matches the entry count", () => { + expect( + summarizeStringEntries({ + entries: ["a", "b", "c"], + limit: 3, + emptyText: "ignored", + }), + ).toBe("a, b, c"); + }); }); diff --git a/src/shared/tailscale-status.test.ts b/src/shared/tailscale-status.test.ts index 5826e4b00b3..94128e700ed 100644 --- a/src/shared/tailscale-status.test.ts +++ b/src/shared/tailscale-status.test.ts @@ -32,6 +32,28 @@ describe("shared/tailscale-status", () => { ); }); + it("falls back to the first tailscale ip when DNSName is blank", async () => { + const run = vi.fn().mockResolvedValue({ + code: 0, + stdout: '{"Self":{"DNSName":"","TailscaleIPs":["100.64.0.10","fd7a::2"]}}', + }); + + await expect(resolveTailnetHostWithRunner(run)).resolves.toBe("100.64.0.10"); + }); + + it("continues to later command candidates when earlier output has no usable host", async () => { + const run = vi + .fn() + .mockResolvedValueOnce({ code: 0, stdout: '{"Self":{}}' }) + .mockResolvedValueOnce({ + code: 0, + stdout: '{"Self":{"DNSName":"backup.tail.ts.net."}}', + }); + + await expect(resolveTailnetHostWithRunner(run)).resolves.toBe("backup.tail.ts.net"); + expect(run).toHaveBeenCalledTimes(2); + }); + it("returns null for non-zero exits, blank output, or invalid json", async () => { const run = vi .fn() diff --git a/src/shared/text-chunking.test.ts b/src/shared/text-chunking.test.ts index be1fb518750..83b0de77ae5 100644 --- a/src/shared/text-chunking.test.ts +++ b/src/shared/text-chunking.test.ts @@ -5,12 +5,14 @@ describe("shared/text-chunking", () => { it("returns empty for blank input and the full text when under limit", () => { expect(chunkTextByBreakResolver("", 10, () => 5)).toEqual([]); expect(chunkTextByBreakResolver("hello", 10, () => 2)).toEqual(["hello"]); + expect(chunkTextByBreakResolver("hello", 0, () => 2)).toEqual(["hello"]); }); it("splits at resolver-provided breakpoints and trims separator boundaries", () => { expect( chunkTextByBreakResolver("alpha beta gamma", 10, (window) => window.lastIndexOf(" ")), ).toEqual(["alpha", "beta gamma"]); + expect(chunkTextByBreakResolver("abcd efgh", 4, () => 4)).toEqual(["abcd", "efgh"]); }); it("falls back to hard limits for invalid break indexes", () => { @@ -28,4 +30,11 @@ describe("shared/text-chunking", () => { chunkTextByBreakResolver("word next", 5, (window) => window.lastIndexOf(" ")), ).toEqual(["word", "next"]); }); + + it("trims trailing whitespace from emitted chunks before continuing", () => { + expect(chunkTextByBreakResolver("abc def", 6, (window) => window.lastIndexOf(" "))).toEqual([ + "abc", + "def", + ]); + }); }); diff --git a/src/shared/text/assistant-visible-text.test.ts b/src/shared/text/assistant-visible-text.test.ts index 234d37b96da..1962a86e371 100644 --- a/src/shared/text/assistant-visible-text.test.ts +++ b/src/shared/text/assistant-visible-text.test.ts @@ -42,8 +42,40 @@ describe("stripAssistantInternalScaffolding", () => { expect(stripAssistantInternalScaffolding(input)).toBe(input); }); + it("keeps relevant-memories tags inside inline code", () => { + const input = "Use `example` literally."; + expect(stripAssistantInternalScaffolding(input)).toBe(input); + }); + it("hides unfinished relevant-memories blocks", () => { const input = ["Hello", "", "internal-only"].join("\n"); expect(stripAssistantInternalScaffolding(input)).toBe("Hello\n"); }); + + it("trims leading whitespace after stripping scaffolding", () => { + const input = [ + "", + "secret", + "", + " ", + "", + "internal note", + "", + " Visible", + ].join("\n"); + expect(stripAssistantInternalScaffolding(input)).toBe("Visible"); + }); + + it("preserves unfinished reasoning text while still stripping memory blocks", () => { + const input = [ + "Before", + "", + "secret", + "", + "internal note", + "", + "After", + ].join("\n"); + expect(stripAssistantInternalScaffolding(input)).toBe("Before\n\nsecret\n\nAfter"); + }); }); diff --git a/src/shared/text/code-regions.test.ts b/src/shared/text/code-regions.test.ts index 05934383bd2..2a9c2a05360 100644 --- a/src/shared/text/code-regions.test.ts +++ b/src/shared/text/code-regions.test.ts @@ -27,13 +27,25 @@ describe("shared/text/code-regions", () => { expect(text.slice(regions[1].start, regions[1].end)).toBe("```\nunterminated"); }); + it("keeps adjacent inline code outside fenced regions", () => { + const text = ["```ts", "const a = 1;", "```", "after `inline` tail"].join("\n"); + + const regions = findCodeRegions(text); + + expect(regions).toHaveLength(2); + expect(text.slice(regions[0].start, regions[0].end)).toContain("```ts"); + expect(text.slice(regions[1].start, regions[1].end)).toBe("`inline`"); + }); + it("reports whether positions are inside discovered regions", () => { const text = "plain `code` done"; const regions = findCodeRegions(text); const codeStart = text.indexOf("code"); const plainStart = text.indexOf("plain"); + const regionEnd = regions[0]?.end ?? -1; expect(isInsideCode(codeStart, regions)).toBe(true); expect(isInsideCode(plainStart, regions)).toBe(false); + expect(isInsideCode(regionEnd, regions)).toBe(false); }); }); diff --git a/src/slack/monitor/events/messages.test.ts b/src/slack/monitor/events/messages.test.ts index 922458a40b1..25fdb77c025 100644 --- a/src/slack/monitor/events/messages.test.ts +++ b/src/slack/monitor/events/messages.test.ts @@ -18,6 +18,7 @@ vi.mock("../../../pairing/pairing-store.js", () => ({ type MessageHandler = (args: { event: Record; body: unknown }) => Promise; type AppMentionHandler = MessageHandler; +type RegisteredEventName = "message" | "app_mention"; type MessageCase = { overrides?: SlackSystemEventTestOverrides; @@ -25,7 +26,7 @@ type MessageCase = { body?: unknown; }; -function createMessageHandlers(overrides?: SlackSystemEventTestOverrides) { +function createHandlers(eventName: RegisteredEventName, overrides?: SlackSystemEventTestOverrides) { const harness = createSlackSystemEventTestHarness(overrides); const handleSlackMessage = vi.fn(async () => {}); registerSlackMessageEvents({ @@ -33,22 +34,14 @@ function createMessageHandlers(overrides?: SlackSystemEventTestOverrides) { handleSlackMessage, }); return { - handler: harness.getHandler("message") as MessageHandler | null, + handler: harness.getHandler(eventName) as MessageHandler | null, handleSlackMessage, }; } -function createAppMentionHandlers(overrides?: SlackSystemEventTestOverrides) { - const harness = createSlackSystemEventTestHarness(overrides); - const handleSlackMessage = vi.fn(async () => {}); - registerSlackMessageEvents({ - ctx: harness.ctx, - handleSlackMessage, - }); - return { - handler: harness.getHandler("app_mention") as AppMentionHandler | null, - handleSlackMessage, - }; +function resetMessageMocks(): void { + messageQueueMock.mockClear(); + messageAllowMock.mockReset().mockResolvedValue([]); } function makeChangedEvent(overrides?: { channel?: string; user?: string }) { @@ -89,10 +82,40 @@ function makeThreadBroadcastEvent(overrides?: { channel?: string; user?: string }; } +function makeAppMentionEvent(overrides?: { + channel?: string; + channelType?: "channel" | "group" | "im" | "mpim"; + ts?: string; +}) { + return { + type: "app_mention", + channel: overrides?.channel ?? "C123", + channel_type: overrides?.channelType ?? "channel", + user: "U1", + text: "<@U_BOT> hello", + ts: overrides?.ts ?? "123.456", + }; +} + +async function invokeRegisteredHandler(input: { + eventName: RegisteredEventName; + overrides?: SlackSystemEventTestOverrides; + event: Record; + body?: unknown; +}) { + resetMessageMocks(); + const { handler, handleSlackMessage } = createHandlers(input.eventName, input.overrides); + expect(handler).toBeTruthy(); + await handler!({ + event: input.event, + body: input.body ?? {}, + }); + return { handleSlackMessage }; +} + async function runMessageCase(input: MessageCase = {}): Promise { - messageQueueMock.mockClear(); - messageAllowMock.mockReset().mockResolvedValue([]); - const { handler } = createMessageHandlers(input.overrides); + resetMessageMocks(); + const { handler } = createHandlers("message", input.overrides); expect(handler).toBeTruthy(); await handler!({ event: (input.event ?? makeChangedEvent()) as Record, @@ -151,12 +174,9 @@ describe("registerSlackMessageEvents", () => { }); it("passes regular message events to the message handler", async () => { - messageQueueMock.mockClear(); - messageAllowMock.mockReset().mockResolvedValue([]); - const { handler, handleSlackMessage } = createMessageHandlers({ dmPolicy: "open" }); - expect(handler).toBeTruthy(); - - await handler!({ + const { handleSlackMessage } = await invokeRegisteredHandler({ + eventName: "message", + overrides: { dmPolicy: "open" }, event: { type: "message", channel: "D1", @@ -164,7 +184,6 @@ describe("registerSlackMessageEvents", () => { text: "hello", ts: "123.456", }, - body: {}, }); expect(handleSlackMessage).toHaveBeenCalledTimes(1); @@ -172,9 +191,8 @@ describe("registerSlackMessageEvents", () => { }); it("handles channel and group messages via the unified message handler", async () => { - messageQueueMock.mockClear(); - messageAllowMock.mockReset().mockResolvedValue([]); - const { handler, handleSlackMessage } = createMessageHandlers({ + resetMessageMocks(); + const { handler, handleSlackMessage } = createHandlers("message", { dmPolicy: "open", channelType: "channel", }); @@ -206,23 +224,18 @@ describe("registerSlackMessageEvents", () => { }); it("applies subtype system-event handling for channel messages", async () => { - messageQueueMock.mockClear(); - messageAllowMock.mockReset().mockResolvedValue([]); - const { handler, handleSlackMessage } = createMessageHandlers({ - dmPolicy: "open", - channelType: "channel", - }); - - expect(handler).toBeTruthy(); - // message_changed events from channels arrive via the generic "message" // handler with channel_type:"channel" — not a separate event type. - await handler!({ + const { handleSlackMessage } = await invokeRegisteredHandler({ + eventName: "message", + overrides: { + dmPolicy: "open", + channelType: "channel", + }, event: { ...makeChangedEvent({ channel: "C1", user: "U1" }), channel_type: "channel", }, - body: {}, }); expect(handleSlackMessage).not.toHaveBeenCalled(); @@ -230,38 +243,20 @@ describe("registerSlackMessageEvents", () => { }); it("skips app_mention events for DM channel ids even with contradictory channel_type", async () => { - const { handler, handleSlackMessage } = createAppMentionHandlers({ dmPolicy: "open" }); - expect(handler).toBeTruthy(); - - await handler!({ - event: { - type: "app_mention", - channel: "D123", - channel_type: "channel", - user: "U1", - text: "<@U_BOT> hello", - ts: "123.456", - }, - body: {}, + const { handleSlackMessage } = await invokeRegisteredHandler({ + eventName: "app_mention", + overrides: { dmPolicy: "open" }, + event: makeAppMentionEvent({ channel: "D123", channelType: "channel" }), }); expect(handleSlackMessage).not.toHaveBeenCalled(); }); it("routes app_mention events from channels to the message handler", async () => { - const { handler, handleSlackMessage } = createAppMentionHandlers({ dmPolicy: "open" }); - expect(handler).toBeTruthy(); - - await handler!({ - event: { - type: "app_mention", - channel: "C123", - channel_type: "channel", - user: "U1", - text: "<@U_BOT> hello", - ts: "123.789", - }, - body: {}, + const { handleSlackMessage } = await invokeRegisteredHandler({ + eventName: "app_mention", + overrides: { dmPolicy: "open" }, + event: makeAppMentionEvent({ channel: "C123", channelType: "channel", ts: "123.789" }), }); expect(handleSlackMessage).toHaveBeenCalledTimes(1); diff --git a/src/utils.ts b/src/utils.ts index 38c26605b19..caf5edb1969 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -4,8 +4,8 @@ import path from "node:path"; import { resolveOAuthDir } from "./config/paths.js"; import { logVerbose, shouldLogVerbose } from "./globals.js"; import { - expandHomePrefix, resolveEffectiveHomeDir, + resolveHomeRelativePath, resolveRequiredHomeDir, } from "./infra/home-dir.js"; import { isPlainObject } from "./infra/plain-object.js"; @@ -279,19 +279,7 @@ export function resolveUserPath( if (!input) { return ""; } - const trimmed = input.trim(); - if (!trimmed) { - return trimmed; - } - if (trimmed.startsWith("~")) { - const expanded = expandHomePrefix(trimmed, { - home: resolveRequiredHomeDir(env, homedir), - env, - homedir, - }); - return path.resolve(expanded); - } - return path.resolve(trimmed); + return resolveHomeRelativePath(input, { env, homedir }); } export function resolveConfigDir(