From c355b8a6710d8376cf6e46a9c523f10f9a626bea Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:10:37 +0000 Subject: [PATCH 01/12] test: extract message action context coverage --- ... => message-action-runner.context.test.ts} | 358 +++++++++--------- 1 file changed, 171 insertions(+), 187 deletions(-) rename src/infra/outbound/{message-action-runner.test.ts => message-action-runner.context.test.ts} (62%) diff --git a/src/infra/outbound/message-action-runner.test.ts b/src/infra/outbound/message-action-runner.context.test.ts similarity index 62% rename from src/infra/outbound/message-action-runner.test.ts rename to src/infra/outbound/message-action-runner.context.test.ts index 3858bae845e..185ff2bf648 100644 --- a/src/infra/outbound/message-action-runner.test.ts +++ b/src/infra/outbound/message-action-runner.context.test.ts @@ -111,8 +111,9 @@ describe("runMessageAction context isolation", () => { setActivePluginRegistry(createTestRegistry([])); }); - it("allows send when target matches current channel", async () => { - const result = await runDrySend({ + it.each([ + { + name: "allows send when target matches current channel", cfg: slackConfig, actionParams: { channel: "slack", @@ -120,39 +121,27 @@ describe("runMessageAction context isolation", () => { message: "hi", }, toolContext: { currentChannelId: "C12345678" }, - }); - - expect(result.kind).toBe("send"); - }); - - it("accepts legacy to parameter for send", async () => { - const result = await runDrySend({ + }, + { + name: "accepts legacy to parameter for send", cfg: slackConfig, actionParams: { channel: "slack", to: "#C12345678", message: "hi", }, - }); - - expect(result.kind).toBe("send"); - }); - - it("defaults to current channel when target is omitted", async () => { - const result = await runDrySend({ + }, + { + name: "defaults to current channel when target is omitted", cfg: slackConfig, actionParams: { channel: "slack", message: "hi", }, toolContext: { currentChannelId: "C12345678" }, - }); - - expect(result.kind).toBe("send"); - }); - - it("allows media-only send when target matches current channel", async () => { - const result = await runDrySend({ + }, + { + name: "allows media-only send when target matches current channel", cfg: slackConfig, actionParams: { channel: "slack", @@ -160,6 +149,25 @@ describe("runMessageAction context isolation", () => { media: "https://example.com/note.ogg", }, toolContext: { currentChannelId: "C12345678" }, + }, + { + name: "allows send when poll booleans are explicitly false", + cfg: slackConfig, + actionParams: { + channel: "slack", + target: "#C12345678", + message: "hi", + pollMulti: false, + pollAnonymous: false, + pollPublic: false, + }, + toolContext: { currentChannelId: "C12345678" }, + }, + ])("$name", async ({ cfg, actionParams, toolContext }) => { + const result = await runDrySend({ + cfg, + actionParams, + ...(toolContext ? { toolContext } : {}), }); expect(result.kind).toBe("send"); @@ -178,144 +186,111 @@ describe("runMessageAction context isolation", () => { ).rejects.toThrow(/message required/i); }); - it("rejects send actions that include poll creation params", async () => { - await expect( - runDrySend({ - cfg: slackConfig, - actionParams: { - channel: "slack", - target: "#C12345678", - message: "hi", - pollQuestion: "Ready?", - pollOption: ["Yes", "No"], - }, - toolContext: { currentChannelId: "C12345678" }, - }), - ).rejects.toThrow(/use action "poll" instead of "send"/i); - }); - - it("rejects send actions that include string-encoded poll params", async () => { - await expect( - runDrySend({ - cfg: slackConfig, - actionParams: { - channel: "slack", - target: "#C12345678", - message: "hi", - pollDurationSeconds: "60", - pollPublic: "true", - }, - toolContext: { currentChannelId: "C12345678" }, - }), - ).rejects.toThrow(/use action "poll" instead of "send"/i); - }); - - it("rejects send actions that include snake_case poll params", async () => { - await expect( - runDrySend({ - cfg: slackConfig, - actionParams: { - channel: "slack", - target: "#C12345678", - message: "hi", - poll_question: "Ready?", - poll_option: ["Yes", "No"], - poll_public: "true", - }, - toolContext: { currentChannelId: "C12345678" }, - }), - ).rejects.toThrow(/use action "poll" instead of "send"/i); - }); - - it("allows send when poll booleans are explicitly false", async () => { - const result = await runDrySend({ - cfg: slackConfig, + it.each([ + { + name: "structured poll params", actionParams: { channel: "slack", target: "#C12345678", message: "hi", - pollMulti: false, - pollAnonymous: false, - pollPublic: false, + pollQuestion: "Ready?", + pollOption: ["Yes", "No"], }, - toolContext: { currentChannelId: "C12345678" }, - }); - - expect(result.kind).toBe("send"); - }); - - it("blocks send when target differs from current channel", async () => { - const result = await runDrySend({ - cfg: slackConfig, + }, + { + name: "string-encoded poll params", actionParams: { channel: "slack", - target: "channel:C99999999", + target: "#C12345678", message: "hi", + pollDurationSeconds: "60", + pollPublic: "true", }, - toolContext: { currentChannelId: "C12345678", currentChannelProvider: "slack" }, - }); - - expect(result.kind).toBe("send"); - }); - - it("blocks thread-reply when channelId differs from current channel", async () => { - const result = await runDryAction({ - cfg: slackConfig, - action: "thread-reply", + }, + { + name: "snake_case poll params", actionParams: { channel: "slack", - target: "C99999999", + target: "#C12345678", message: "hi", + poll_question: "Ready?", + poll_option: ["Yes", "No"], + poll_public: "true", }, - toolContext: { currentChannelId: "C12345678", currentChannelProvider: "slack" }, - }); - - expect(result.kind).toBe("action"); + }, + ])("rejects send actions that include $name", async ({ actionParams }) => { + await expect( + runDrySend({ + cfg: slackConfig, + actionParams, + toolContext: { currentChannelId: "C12345678" }, + }), + ).rejects.toThrow(/use action "poll" instead of "send"/i); }); it.each([ { - name: "whatsapp", + name: "send when target differs from current slack channel", + run: () => + runDrySend({ + cfg: slackConfig, + actionParams: { + channel: "slack", + target: "channel:C99999999", + message: "hi", + }, + toolContext: { currentChannelId: "C12345678", currentChannelProvider: "slack" }, + }), + expectedKind: "send", + }, + { + name: "thread-reply when channelId differs from current slack channel", + run: () => + runDryAction({ + cfg: slackConfig, + action: "thread-reply", + actionParams: { + channel: "slack", + target: "C99999999", + message: "hi", + }, + toolContext: { currentChannelId: "C12345678", currentChannelProvider: "slack" }, + }), + expectedKind: "action", + }, + ])("blocks cross-context UI handoff for $name", async ({ run, expectedKind }) => { + const result = await run(); + expect(result.kind).toBe(expectedKind); + }); + + it.each([ + { + name: "whatsapp match", channel: "whatsapp", target: "123@g.us", currentChannelId: "123@g.us", }, { - name: "imessage", + name: "imessage match", channel: "imessage", target: "imessage:+15551234567", currentChannelId: "imessage:+15551234567", }, - ] as const)("allows $name send when target matches current context", async (testCase) => { - const result = await runDrySend({ - cfg: whatsappConfig, - actionParams: { - channel: testCase.channel, - target: testCase.target, - message: "hi", - }, - toolContext: { currentChannelId: testCase.currentChannelId }, - }); - - expect(result.kind).toBe("send"); - }); - - it.each([ { - name: "whatsapp", + name: "whatsapp mismatch", channel: "whatsapp", target: "456@g.us", currentChannelId: "123@g.us", currentChannelProvider: "whatsapp", }, { - name: "imessage", + name: "imessage mismatch", channel: "imessage", target: "imessage:+15551230000", currentChannelId: "imessage:+15551234567", currentChannelProvider: "imessage", }, - ] as const)("blocks $name send when target differs from current context", async (testCase) => { + ] as const)("$name", async (testCase) => { const result = await runDrySend({ cfg: whatsappConfig, actionParams: { @@ -325,106 +300,115 @@ describe("runMessageAction context isolation", () => { }, toolContext: { currentChannelId: testCase.currentChannelId, - currentChannelProvider: testCase.currentChannelProvider, + ...(testCase.currentChannelProvider + ? { currentChannelProvider: testCase.currentChannelProvider } + : {}), }, }); expect(result.kind).toBe("send"); }); - it("infers channel + target from tool context when missing", async () => { - const multiConfig = { - channels: { - slack: { - botToken: "xoxb-test", - appToken: "xapp-test", + it.each([ + { + name: "infers channel + target from tool context when missing", + cfg: { + channels: { + slack: { + botToken: "xoxb-test", + appToken: "xapp-test", + }, + telegram: { + token: "tg-test", + }, }, - telegram: { - token: "tg-test", - }, - }, - } as OpenClawConfig; - - const result = await runDrySend({ - cfg: multiConfig, + } as OpenClawConfig, + action: "send" as const, actionParams: { message: "hi", }, toolContext: { currentChannelId: "C12345678", currentChannelProvider: "slack" }, - }); - - expect(result.kind).toBe("send"); - expect(result.channel).toBe("slack"); - }); - - it("falls back to tool-context provider when channel param is an id", async () => { - const result = await runDrySend({ + expectedKind: "send", + expectedChannel: "slack", + }, + { + name: "falls back to tool-context provider when channel param is an id", cfg: slackConfig, + action: "send" as const, actionParams: { channel: "C12345678", target: "#C12345678", message: "hi", }, toolContext: { currentChannelId: "C12345678", currentChannelProvider: "slack" }, - }); - - expect(result.kind).toBe("send"); - expect(result.channel).toBe("slack"); - }); - - it("falls back to tool-context provider for broadcast channel ids", async () => { - const result = await runDryAction({ + expectedKind: "send", + expectedChannel: "slack", + }, + { + name: "falls back to tool-context provider for broadcast channel ids", cfg: slackConfig, - action: "broadcast", + action: "broadcast" as const, actionParams: { targets: ["channel:C12345678"], channel: "C12345678", message: "hi", }, toolContext: { currentChannelProvider: "slack" }, + expectedKind: "broadcast", + expectedChannel: "slack", + }, + ])("$name", async ({ cfg, action, actionParams, toolContext, expectedKind, expectedChannel }) => { + const result = await runDryAction({ + cfg, + action, + actionParams, + toolContext, }); - expect(result.kind).toBe("broadcast"); - expect(result.channel).toBe("slack"); + expect(result.kind).toBe(expectedKind); + expect(result.channel).toBe(expectedChannel); }); - it("blocks cross-provider sends by default", async () => { - await expect( - runDrySend({ - cfg: slackConfig, - actionParams: { - channel: "telegram", - target: "@opsbot", - message: "hi", - }, - toolContext: { currentChannelId: "C12345678", currentChannelProvider: "slack" }, - }), - ).rejects.toThrow(/Cross-context messaging denied/); - }); - - it("blocks same-provider cross-context when disabled", async () => { - const cfg = { - ...slackConfig, - tools: { - message: { - crossContext: { - allowWithinProvider: false, + it.each([ + { + name: "blocks cross-provider sends by default", + cfg: slackConfig, + actionParams: { + channel: "telegram", + target: "@opsbot", + message: "hi", + }, + toolContext: { currentChannelId: "C12345678", currentChannelProvider: "slack" }, + message: /Cross-context messaging denied/, + }, + { + name: "blocks same-provider cross-context when disabled", + cfg: { + ...slackConfig, + tools: { + message: { + crossContext: { + allowWithinProvider: false, + }, }, }, + } as OpenClawConfig, + actionParams: { + channel: "slack", + target: "channel:C99999999", + message: "hi", }, - } as OpenClawConfig; - + toolContext: { currentChannelId: "C12345678", currentChannelProvider: "slack" }, + message: /Cross-context messaging denied/, + }, + ])("$name", async ({ cfg, actionParams, toolContext, message }) => { await expect( runDrySend({ cfg, - actionParams: { - channel: "slack", - target: "channel:C99999999", - message: "hi", - }, - toolContext: { currentChannelId: "C12345678", currentChannelProvider: "slack" }, + actionParams, + toolContext, }), - ).rejects.toThrow(/Cross-context messaging denied/); + ).rejects.toThrow(message); }); it.each([ From c2a9c5699dca19e5f76346673b263bccafe59873 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:13:52 +0000 Subject: [PATCH 02/12] test: extract outbound delivery lifecycle coverage --- src/infra/outbound/deliver.lifecycle.test.ts | 415 +++++++++++++++++++ src/infra/outbound/deliver.test.ts | 314 +------------- 2 files changed, 429 insertions(+), 300 deletions(-) create mode 100644 src/infra/outbound/deliver.lifecycle.test.ts diff --git a/src/infra/outbound/deliver.lifecycle.test.ts b/src/infra/outbound/deliver.lifecycle.test.ts new file mode 100644 index 00000000000..00d696162d8 --- /dev/null +++ b/src/infra/outbound/deliver.lifecycle.test.ts @@ -0,0 +1,415 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { signalOutbound } from "../../channels/plugins/outbound/signal.js"; +import { telegramOutbound } from "../../channels/plugins/outbound/telegram.js"; +import { whatsappOutbound } from "../../channels/plugins/outbound/whatsapp.js"; +import type { OpenClawConfig } from "../../config/config.js"; +import { setActivePluginRegistry } from "../../plugins/runtime.js"; +import { createOutboundTestPlugin, createTestRegistry } from "../../test-utils/channel-plugins.js"; +import { createIMessageTestPlugin } from "../../test-utils/imessage-test-plugin.js"; +import { createInternalHookEventPayload } from "../../test-utils/internal-hook-event-payload.js"; + +const mocks = vi.hoisted(() => ({ + appendAssistantMessageToSessionTranscript: vi.fn(async () => ({ ok: true, sessionFile: "x" })), +})); +const hookMocks = vi.hoisted(() => ({ + runner: { + hasHooks: vi.fn(() => false), + runMessageSent: vi.fn(async () => {}), + }, +})); +const internalHookMocks = vi.hoisted(() => ({ + createInternalHookEvent: vi.fn(), + triggerInternalHook: vi.fn(async () => {}), +})); +const queueMocks = vi.hoisted(() => ({ + enqueueDelivery: vi.fn(async () => "mock-queue-id"), + ackDelivery: vi.fn(async () => {}), + failDelivery: vi.fn(async () => {}), +})); +const logMocks = vi.hoisted(() => ({ + warn: vi.fn(), +})); + +vi.mock("../../config/sessions.js", async () => { + const actual = await vi.importActual( + "../../config/sessions.js", + ); + return { + ...actual, + appendAssistantMessageToSessionTranscript: mocks.appendAssistantMessageToSessionTranscript, + }; +}); +vi.mock("../../plugins/hook-runner-global.js", () => ({ + getGlobalHookRunner: () => hookMocks.runner, +})); +vi.mock("../../hooks/internal-hooks.js", () => ({ + createInternalHookEvent: internalHookMocks.createInternalHookEvent, + triggerInternalHook: internalHookMocks.triggerInternalHook, +})); +vi.mock("./delivery-queue.js", () => ({ + enqueueDelivery: queueMocks.enqueueDelivery, + ackDelivery: queueMocks.ackDelivery, + failDelivery: queueMocks.failDelivery, +})); +vi.mock("../../logging/subsystem.js", () => ({ + createSubsystemLogger: () => { + const makeLogger = () => ({ + warn: logMocks.warn, + info: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + child: vi.fn(() => makeLogger()), + }); + return makeLogger(); + }, +})); + +const { deliverOutboundPayloads } = await import("./deliver.js"); + +const whatsappChunkConfig: OpenClawConfig = { + channels: { whatsapp: { textChunkLimit: 4000 } }, +}; + +async function runChunkedWhatsAppDelivery(params?: { + mirror?: Parameters[0]["mirror"]; +}) { + const sendWhatsApp = vi + .fn() + .mockResolvedValueOnce({ messageId: "w1", toJid: "jid" }) + .mockResolvedValueOnce({ messageId: "w2", toJid: "jid" }); + const cfg: OpenClawConfig = { + channels: { whatsapp: { textChunkLimit: 2 } }, + }; + const results = await deliverOutboundPayloads({ + cfg, + channel: "whatsapp", + to: "+1555", + payloads: [{ text: "abcd" }], + deps: { sendWhatsApp }, + ...(params?.mirror ? { mirror: params.mirror } : {}), + }); + return { sendWhatsApp, results }; +} + +async function deliverSingleWhatsAppForHookTest(params?: { sessionKey?: string }) { + const sendWhatsApp = vi.fn().mockResolvedValue({ messageId: "w1", toJid: "jid" }); + await deliverOutboundPayloads({ + cfg: whatsappChunkConfig, + channel: "whatsapp", + to: "+1555", + payloads: [{ text: "hello" }], + deps: { sendWhatsApp }, + ...(params?.sessionKey ? { session: { key: params.sessionKey } } : {}), + }); +} + +async function runBestEffortPartialFailureDelivery() { + const sendWhatsApp = vi + .fn() + .mockRejectedValueOnce(new Error("fail")) + .mockResolvedValueOnce({ messageId: "w2", toJid: "jid" }); + const onError = vi.fn(); + const cfg: OpenClawConfig = {}; + const results = await deliverOutboundPayloads({ + cfg, + channel: "whatsapp", + to: "+1555", + payloads: [{ text: "a" }, { text: "b" }], + deps: { sendWhatsApp }, + bestEffort: true, + onError, + }); + return { sendWhatsApp, onError, results }; +} + +function expectSuccessfulWhatsAppInternalHookPayload( + expected: Partial<{ + content: string; + messageId: string; + isGroup: boolean; + groupId: string; + }>, +) { + return expect.objectContaining({ + to: "+1555", + success: true, + channelId: "whatsapp", + conversationId: "+1555", + ...expected, + }); +} + +describe("deliverOutboundPayloads lifecycle", () => { + beforeEach(() => { + setActivePluginRegistry(defaultRegistry); + hookMocks.runner.hasHooks.mockClear(); + hookMocks.runner.hasHooks.mockReturnValue(false); + hookMocks.runner.runMessageSent.mockClear(); + hookMocks.runner.runMessageSent.mockResolvedValue(undefined); + internalHookMocks.createInternalHookEvent.mockClear(); + internalHookMocks.createInternalHookEvent.mockImplementation(createInternalHookEventPayload); + internalHookMocks.triggerInternalHook.mockClear(); + queueMocks.enqueueDelivery.mockClear(); + queueMocks.enqueueDelivery.mockResolvedValue("mock-queue-id"); + queueMocks.ackDelivery.mockClear(); + queueMocks.ackDelivery.mockResolvedValue(undefined); + queueMocks.failDelivery.mockClear(); + queueMocks.failDelivery.mockResolvedValue(undefined); + logMocks.warn.mockClear(); + mocks.appendAssistantMessageToSessionTranscript.mockClear(); + }); + + afterEach(() => { + setActivePluginRegistry(emptyRegistry); + }); + + it("continues on errors when bestEffort is enabled", async () => { + const { sendWhatsApp, onError, results } = await runBestEffortPartialFailureDelivery(); + + expect(sendWhatsApp).toHaveBeenCalledTimes(2); + expect(onError).toHaveBeenCalledTimes(1); + expect(results).toEqual([{ channel: "whatsapp", messageId: "w2", toJid: "jid" }]); + }); + + it("calls failDelivery instead of ackDelivery on bestEffort partial failure", async () => { + const { onError } = await runBestEffortPartialFailureDelivery(); + + expect(onError).toHaveBeenCalledTimes(1); + expect(queueMocks.ackDelivery).not.toHaveBeenCalled(); + expect(queueMocks.failDelivery).toHaveBeenCalledWith( + "mock-queue-id", + "partial delivery failure (bestEffort)", + ); + }); + + it("passes normalized payload to onError", async () => { + const sendWhatsApp = vi.fn().mockRejectedValue(new Error("boom")); + const onError = vi.fn(); + + await deliverOutboundPayloads({ + cfg: {}, + channel: "whatsapp", + to: "+1555", + payloads: [{ text: "hi", mediaUrl: "https://x.test/a.jpg" }], + deps: { sendWhatsApp }, + bestEffort: true, + onError, + }); + + expect(onError).toHaveBeenCalledTimes(1); + expect(onError).toHaveBeenCalledWith( + expect.any(Error), + expect.objectContaining({ text: "hi", mediaUrls: ["https://x.test/a.jpg"] }), + ); + }); + + it("acks the queue entry when delivery is aborted", async () => { + const sendWhatsApp = vi.fn().mockResolvedValue({ messageId: "w1", toJid: "jid" }); + const abortController = new AbortController(); + abortController.abort(); + + await expect( + deliverOutboundPayloads({ + cfg: {}, + channel: "whatsapp", + to: "+1555", + payloads: [{ text: "a" }], + deps: { sendWhatsApp }, + abortSignal: abortController.signal, + }), + ).rejects.toThrow("Operation aborted"); + + expect(queueMocks.ackDelivery).toHaveBeenCalledWith("mock-queue-id"); + expect(queueMocks.failDelivery).not.toHaveBeenCalled(); + expect(sendWhatsApp).not.toHaveBeenCalled(); + }); + + it("emits internal message:sent hook with success=true for chunked payload delivery", async () => { + const { sendWhatsApp } = await runChunkedWhatsAppDelivery({ + mirror: { + sessionKey: "agent:main:main", + isGroup: true, + groupId: "whatsapp:group:123", + }, + }); + expect(sendWhatsApp).toHaveBeenCalledTimes(2); + + expect(internalHookMocks.createInternalHookEvent).toHaveBeenCalledTimes(1); + expect(internalHookMocks.createInternalHookEvent).toHaveBeenCalledWith( + "message", + "sent", + "agent:main:main", + expectSuccessfulWhatsAppInternalHookPayload({ + content: "abcd", + messageId: "w2", + isGroup: true, + groupId: "whatsapp:group:123", + }), + ); + expect(internalHookMocks.triggerInternalHook).toHaveBeenCalledTimes(1); + }); + + it("does not emit internal message:sent hook when neither mirror nor sessionKey is provided", async () => { + await deliverSingleWhatsAppForHookTest(); + + expect(internalHookMocks.createInternalHookEvent).not.toHaveBeenCalled(); + expect(internalHookMocks.triggerInternalHook).not.toHaveBeenCalled(); + }); + + it("emits internal message:sent hook when sessionKey is provided without mirror", async () => { + await deliverSingleWhatsAppForHookTest({ sessionKey: "agent:main:main" }); + + expect(internalHookMocks.createInternalHookEvent).toHaveBeenCalledTimes(1); + expect(internalHookMocks.createInternalHookEvent).toHaveBeenCalledWith( + "message", + "sent", + "agent:main:main", + expectSuccessfulWhatsAppInternalHookPayload({ content: "hello", messageId: "w1" }), + ); + expect(internalHookMocks.triggerInternalHook).toHaveBeenCalledTimes(1); + }); + + it("warns when session.agentId is set without a session key", async () => { + const sendWhatsApp = vi.fn().mockResolvedValue({ messageId: "w1", toJid: "jid" }); + hookMocks.runner.hasHooks.mockReturnValue(true); + + await deliverOutboundPayloads({ + cfg: whatsappChunkConfig, + channel: "whatsapp", + to: "+1555", + payloads: [{ text: "hello" }], + deps: { sendWhatsApp }, + session: { agentId: "agent-main" }, + }); + + expect(logMocks.warn).toHaveBeenCalledWith( + "deliverOutboundPayloads: session.agentId present without session key; internal message:sent hook will be skipped", + expect.objectContaining({ channel: "whatsapp", to: "+1555", agentId: "agent-main" }), + ); + }); + + it("mirrors delivered output when mirror options are provided", async () => { + const sendTelegram = vi.fn().mockResolvedValue({ messageId: "m1", chatId: "c1" }); + + await deliverOutboundPayloads({ + cfg: { + channels: { telegram: { botToken: "tok-1", textChunkLimit: 2 } }, + }, + channel: "telegram", + to: "123", + payloads: [{ text: "caption", mediaUrl: "https://example.com/files/report.pdf?sig=1" }], + deps: { sendTelegram }, + mirror: { + sessionKey: "agent:main:main", + text: "caption", + mediaUrls: ["https://example.com/files/report.pdf?sig=1"], + idempotencyKey: "idem-deliver-1", + }, + }); + + expect(mocks.appendAssistantMessageToSessionTranscript).toHaveBeenCalledWith( + expect.objectContaining({ + text: "report.pdf", + idempotencyKey: "idem-deliver-1", + }), + ); + }); + + it("emits message_sent success for text-only deliveries", async () => { + hookMocks.runner.hasHooks.mockReturnValue(true); + const sendWhatsApp = vi.fn().mockResolvedValue({ messageId: "w1", toJid: "jid" }); + + await deliverOutboundPayloads({ + cfg: {}, + channel: "whatsapp", + to: "+1555", + payloads: [{ text: "hello" }], + deps: { sendWhatsApp }, + }); + + expect(hookMocks.runner.runMessageSent).toHaveBeenCalledWith( + expect.objectContaining({ to: "+1555", content: "hello", success: true }), + expect.objectContaining({ channelId: "whatsapp" }), + ); + }); + + it("emits message_sent success for sendPayload deliveries", async () => { + hookMocks.runner.hasHooks.mockReturnValue(true); + const sendPayload = vi.fn().mockResolvedValue({ channel: "matrix", messageId: "mx-1" }); + const sendText = vi.fn(); + const sendMedia = vi.fn(); + setActivePluginRegistry( + createTestRegistry([ + { + pluginId: "matrix", + source: "test", + plugin: createOutboundTestPlugin({ + id: "matrix", + outbound: { deliveryMode: "direct", sendPayload, sendText, sendMedia }, + }), + }, + ]), + ); + + await deliverOutboundPayloads({ + cfg: {}, + channel: "matrix", + to: "!room:1", + payloads: [{ text: "payload text", channelData: { mode: "custom" } }], + }); + + expect(hookMocks.runner.runMessageSent).toHaveBeenCalledWith( + expect.objectContaining({ to: "!room:1", content: "payload text", success: true }), + expect.objectContaining({ channelId: "matrix" }), + ); + }); + + it("emits message_sent failure when delivery errors", async () => { + hookMocks.runner.hasHooks.mockReturnValue(true); + const sendWhatsApp = vi.fn().mockRejectedValue(new Error("downstream failed")); + + await expect( + deliverOutboundPayloads({ + cfg: {}, + channel: "whatsapp", + to: "+1555", + payloads: [{ text: "hi" }], + deps: { sendWhatsApp }, + }), + ).rejects.toThrow("downstream failed"); + + expect(hookMocks.runner.runMessageSent).toHaveBeenCalledWith( + expect.objectContaining({ + to: "+1555", + content: "hi", + success: false, + error: "downstream failed", + }), + expect.objectContaining({ channelId: "whatsapp" }), + ); + }); +}); + +const emptyRegistry = createTestRegistry([]); +const defaultRegistry = createTestRegistry([ + { + pluginId: "telegram", + plugin: createOutboundTestPlugin({ id: "telegram", outbound: telegramOutbound }), + source: "test", + }, + { + pluginId: "signal", + plugin: createOutboundTestPlugin({ id: "signal", outbound: signalOutbound }), + source: "test", + }, + { + pluginId: "whatsapp", + plugin: createOutboundTestPlugin({ id: "whatsapp", outbound: whatsappOutbound }), + source: "test", + }, + { + pluginId: "imessage", + plugin: createIMessageTestPlugin(), + source: "test", + }, +]); diff --git a/src/infra/outbound/deliver.test.ts b/src/infra/outbound/deliver.test.ts index 8e5383ea055..223b984382b 100644 --- a/src/infra/outbound/deliver.test.ts +++ b/src/infra/outbound/deliver.test.ts @@ -117,75 +117,6 @@ async function deliverTelegramPayload(params: { }); } -async function runChunkedWhatsAppDelivery(params?: { - mirror?: Parameters[0]["mirror"]; -}) { - const sendWhatsApp = vi - .fn() - .mockResolvedValueOnce({ messageId: "w1", toJid: "jid" }) - .mockResolvedValueOnce({ messageId: "w2", toJid: "jid" }); - const cfg: OpenClawConfig = { - channels: { whatsapp: { textChunkLimit: 2 } }, - }; - const results = await deliverOutboundPayloads({ - cfg, - channel: "whatsapp", - to: "+1555", - payloads: [{ text: "abcd" }], - deps: { sendWhatsApp }, - ...(params?.mirror ? { mirror: params.mirror } : {}), - }); - return { sendWhatsApp, results }; -} - -async function deliverSingleWhatsAppForHookTest(params?: { sessionKey?: string }) { - const sendWhatsApp = vi.fn().mockResolvedValue({ messageId: "w1", toJid: "jid" }); - await deliverOutboundPayloads({ - cfg: whatsappChunkConfig, - channel: "whatsapp", - to: "+1555", - payloads: [{ text: "hello" }], - deps: { sendWhatsApp }, - ...(params?.sessionKey ? { session: { key: params.sessionKey } } : {}), - }); -} - -async function runBestEffortPartialFailureDelivery() { - const sendWhatsApp = vi - .fn() - .mockRejectedValueOnce(new Error("fail")) - .mockResolvedValueOnce({ messageId: "w2", toJid: "jid" }); - const onError = vi.fn(); - const cfg: OpenClawConfig = {}; - const results = await deliverOutboundPayloads({ - cfg, - channel: "whatsapp", - to: "+1555", - payloads: [{ text: "a" }, { text: "b" }], - deps: { sendWhatsApp }, - bestEffort: true, - onError, - }); - return { sendWhatsApp, onError, results }; -} - -function expectSuccessfulWhatsAppInternalHookPayload( - expected: Partial<{ - content: string; - messageId: string; - isGroup: boolean; - groupId: string; - }>, -) { - return expect.objectContaining({ - to: "+1555", - success: true, - channelId: "whatsapp", - conversationId: "+1555", - ...expected, - }); -} - describe("deliverOutboundPayloads", () => { beforeEach(() => { setActivePluginRegistry(defaultRegistry); @@ -529,7 +460,20 @@ describe("deliverOutboundPayloads", () => { }); it("chunks WhatsApp text and returns all results", async () => { - const { sendWhatsApp, results } = await runChunkedWhatsAppDelivery(); + const sendWhatsApp = vi + .fn() + .mockResolvedValueOnce({ messageId: "w1", toJid: "jid" }) + .mockResolvedValueOnce({ messageId: "w2", toJid: "jid" }); + const cfg: OpenClawConfig = { + channels: { whatsapp: { textChunkLimit: 2 } }, + }; + const results = await deliverOutboundPayloads({ + cfg, + channel: "whatsapp", + to: "+1555", + payloads: [{ text: "abcd" }], + deps: { sendWhatsApp }, + }); expect(sendWhatsApp).toHaveBeenCalledTimes(2); expect(results.map((r) => r.messageId)).toEqual(["w1", "w2"]); @@ -725,211 +669,6 @@ describe("deliverOutboundPayloads", () => { ]); }); - it("continues on errors when bestEffort is enabled", async () => { - const { sendWhatsApp, onError, results } = await runBestEffortPartialFailureDelivery(); - - expect(sendWhatsApp).toHaveBeenCalledTimes(2); - expect(onError).toHaveBeenCalledTimes(1); - expect(results).toEqual([{ channel: "whatsapp", messageId: "w2", toJid: "jid" }]); - }); - - it("emits internal message:sent hook with success=true for chunked payload delivery", async () => { - const { sendWhatsApp } = await runChunkedWhatsAppDelivery({ - mirror: { - sessionKey: "agent:main:main", - isGroup: true, - groupId: "whatsapp:group:123", - }, - }); - expect(sendWhatsApp).toHaveBeenCalledTimes(2); - - expect(internalHookMocks.createInternalHookEvent).toHaveBeenCalledTimes(1); - expect(internalHookMocks.createInternalHookEvent).toHaveBeenCalledWith( - "message", - "sent", - "agent:main:main", - expectSuccessfulWhatsAppInternalHookPayload({ - content: "abcd", - messageId: "w2", - isGroup: true, - groupId: "whatsapp:group:123", - }), - ); - expect(internalHookMocks.triggerInternalHook).toHaveBeenCalledTimes(1); - }); - - it("does not emit internal message:sent hook when neither mirror nor sessionKey is provided", async () => { - await deliverSingleWhatsAppForHookTest(); - - expect(internalHookMocks.createInternalHookEvent).not.toHaveBeenCalled(); - expect(internalHookMocks.triggerInternalHook).not.toHaveBeenCalled(); - }); - - it("emits internal message:sent hook when sessionKey is provided without mirror", async () => { - await deliverSingleWhatsAppForHookTest({ sessionKey: "agent:main:main" }); - - expect(internalHookMocks.createInternalHookEvent).toHaveBeenCalledTimes(1); - expect(internalHookMocks.createInternalHookEvent).toHaveBeenCalledWith( - "message", - "sent", - "agent:main:main", - expectSuccessfulWhatsAppInternalHookPayload({ content: "hello", messageId: "w1" }), - ); - expect(internalHookMocks.triggerInternalHook).toHaveBeenCalledTimes(1); - }); - - it("warns when session.agentId is set without a session key", async () => { - const sendWhatsApp = vi.fn().mockResolvedValue({ messageId: "w1", toJid: "jid" }); - hookMocks.runner.hasHooks.mockReturnValue(true); - - await deliverOutboundPayloads({ - cfg: whatsappChunkConfig, - channel: "whatsapp", - to: "+1555", - payloads: [{ text: "hello" }], - deps: { sendWhatsApp }, - session: { agentId: "agent-main" }, - }); - - expect(logMocks.warn).toHaveBeenCalledWith( - "deliverOutboundPayloads: session.agentId present without session key; internal message:sent hook will be skipped", - expect.objectContaining({ channel: "whatsapp", to: "+1555", agentId: "agent-main" }), - ); - }); - - it("calls failDelivery instead of ackDelivery on bestEffort partial failure", async () => { - const { onError } = await runBestEffortPartialFailureDelivery(); - - // onError was called for the first payload's failure. - expect(onError).toHaveBeenCalledTimes(1); - - // Queue entry should NOT be acked — failDelivery should be called instead. - expect(queueMocks.ackDelivery).not.toHaveBeenCalled(); - expect(queueMocks.failDelivery).toHaveBeenCalledWith( - "mock-queue-id", - "partial delivery failure (bestEffort)", - ); - }); - - it("acks the queue entry when delivery is aborted", async () => { - const sendWhatsApp = vi.fn().mockResolvedValue({ messageId: "w1", toJid: "jid" }); - const abortController = new AbortController(); - abortController.abort(); - const cfg: OpenClawConfig = {}; - - await expect( - deliverOutboundPayloads({ - cfg, - channel: "whatsapp", - to: "+1555", - payloads: [{ text: "a" }], - deps: { sendWhatsApp }, - abortSignal: abortController.signal, - }), - ).rejects.toThrow("Operation aborted"); - - expect(queueMocks.ackDelivery).toHaveBeenCalledWith("mock-queue-id"); - expect(queueMocks.failDelivery).not.toHaveBeenCalled(); - expect(sendWhatsApp).not.toHaveBeenCalled(); - }); - - it("passes normalized payload to onError", async () => { - const sendWhatsApp = vi.fn().mockRejectedValue(new Error("boom")); - const onError = vi.fn(); - const cfg: OpenClawConfig = {}; - - await deliverOutboundPayloads({ - cfg, - channel: "whatsapp", - to: "+1555", - payloads: [{ text: "hi", mediaUrl: "https://x.test/a.jpg" }], - deps: { sendWhatsApp }, - bestEffort: true, - onError, - }); - - expect(onError).toHaveBeenCalledTimes(1); - expect(onError).toHaveBeenCalledWith( - expect.any(Error), - expect.objectContaining({ text: "hi", mediaUrls: ["https://x.test/a.jpg"] }), - ); - }); - - it("mirrors delivered output when mirror options are provided", async () => { - const sendTelegram = vi.fn().mockResolvedValue({ messageId: "m1", chatId: "c1" }); - mocks.appendAssistantMessageToSessionTranscript.mockClear(); - - await deliverOutboundPayloads({ - cfg: telegramChunkConfig, - channel: "telegram", - to: "123", - payloads: [{ text: "caption", mediaUrl: "https://example.com/files/report.pdf?sig=1" }], - deps: { sendTelegram }, - mirror: { - sessionKey: "agent:main:main", - text: "caption", - mediaUrls: ["https://example.com/files/report.pdf?sig=1"], - idempotencyKey: "idem-deliver-1", - }, - }); - - expect(mocks.appendAssistantMessageToSessionTranscript).toHaveBeenCalledWith( - expect.objectContaining({ - text: "report.pdf", - idempotencyKey: "idem-deliver-1", - }), - ); - }); - - it("emits message_sent success for text-only deliveries", async () => { - hookMocks.runner.hasHooks.mockReturnValue(true); - const sendWhatsApp = vi.fn().mockResolvedValue({ messageId: "w1", toJid: "jid" }); - - await deliverOutboundPayloads({ - cfg: {}, - channel: "whatsapp", - to: "+1555", - payloads: [{ text: "hello" }], - deps: { sendWhatsApp }, - }); - - expect(hookMocks.runner.runMessageSent).toHaveBeenCalledWith( - expect.objectContaining({ to: "+1555", content: "hello", success: true }), - expect.objectContaining({ channelId: "whatsapp" }), - ); - }); - - it("emits message_sent success for sendPayload deliveries", async () => { - hookMocks.runner.hasHooks.mockReturnValue(true); - const sendPayload = vi.fn().mockResolvedValue({ channel: "matrix", messageId: "mx-1" }); - const sendText = vi.fn(); - const sendMedia = vi.fn(); - setActivePluginRegistry( - createTestRegistry([ - { - pluginId: "matrix", - source: "test", - plugin: createOutboundTestPlugin({ - id: "matrix", - outbound: { deliveryMode: "direct", sendPayload, sendText, sendMedia }, - }), - }, - ]), - ); - - await deliverOutboundPayloads({ - cfg: {}, - channel: "matrix", - to: "!room:1", - payloads: [{ text: "payload text", channelData: { mode: "custom" } }], - }); - - expect(hookMocks.runner.runMessageSent).toHaveBeenCalledWith( - expect.objectContaining({ to: "!room:1", content: "payload text", success: true }), - expect.objectContaining({ channelId: "matrix" }), - ); - }); - it("preserves channelData-only payloads with empty text for non-WhatsApp sendPayload channels", async () => { const sendPayload = vi.fn().mockResolvedValue({ channel: "line", messageId: "ln-1" }); const sendText = vi.fn(); @@ -1090,31 +829,6 @@ describe("deliverOutboundPayloads", () => { expect.objectContaining({ channelId: "matrix" }), ); }); - - it("emits message_sent failure when delivery errors", async () => { - hookMocks.runner.hasHooks.mockReturnValue(true); - const sendWhatsApp = vi.fn().mockRejectedValue(new Error("downstream failed")); - - await expect( - deliverOutboundPayloads({ - cfg: {}, - channel: "whatsapp", - to: "+1555", - payloads: [{ text: "hi" }], - deps: { sendWhatsApp }, - }), - ).rejects.toThrow("downstream failed"); - - expect(hookMocks.runner.runMessageSent).toHaveBeenCalledWith( - expect.objectContaining({ - to: "+1555", - content: "hi", - success: false, - error: "downstream failed", - }), - expect.objectContaining({ channelId: "whatsapp" }), - ); - }); }); const emptyRegistry = createTestRegistry([]); From 8c7bdbe4d1aac5c034cc13b9f2fb3a9be716d390 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Fri, 13 Mar 2026 17:16:14 -0400 Subject: [PATCH 03/12] Docs: describe Slack interactive replies (#45463) --- docs/channels/slack.md | 49 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/docs/channels/slack.md b/docs/channels/slack.md index 7fe44cc611b..aa9127ea630 100644 --- a/docs/channels/slack.md +++ b/docs/channels/slack.md @@ -218,6 +218,55 @@ For actions/directory reads, user token can be preferred when configured. For wr - if encoded option values exceed Slack limits, the flow falls back to buttons - For long option payloads, Slash command argument menus use a confirm dialog before dispatching a selected value. +## Interactive replies + +Slack can render agent-authored interactive reply controls, but this feature is disabled by default. + +Enable it globally: + +```json5 +{ + channels: { + slack: { + capabilities: { + interactiveReplies: true, + }, + }, + }, +} +``` + +Or enable it for one Slack account only: + +```json5 +{ + channels: { + slack: { + accounts: { + ops: { + capabilities: { + interactiveReplies: true, + }, + }, + }, + }, + }, +} +``` + +When enabled, agents can emit Slack-only reply directives: + +- `[[slack_buttons: Approve:approve, Reject:reject]]` +- `[[slack_select: Choose a target | Canary:canary, Production:production]]` + +These directives compile into Slack Block Kit and route clicks or selections back through the existing Slack interaction event path. + +Notes: + +- This is Slack-specific UI. Other channels do not translate Slack Block Kit directives into their own button systems. +- The interactive callback values are OpenClaw-generated opaque tokens, not raw agent-authored values. +- If generated interactive blocks would exceed Slack Block Kit limits, OpenClaw falls back to the original text reply instead of sending an invalid blocks payload. + Default slash command settings: - `enabled: false` From d062252522d7198c0370b5c1bbabddaff87a2c59 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:18:13 +0000 Subject: [PATCH 04/12] test: dedupe exec approvals analysis coverage --- src/infra/exec-approvals.test.ts | 308 +------------------------------ 1 file changed, 2 insertions(+), 306 deletions(-) diff --git a/src/infra/exec-approvals.test.ts b/src/infra/exec-approvals.test.ts index 4e49ac10a7b..75cf2b115b6 100644 --- a/src/infra/exec-approvals.test.ts +++ b/src/infra/exec-approvals.test.ts @@ -1,310 +1,6 @@ -import fs from "node:fs"; -import path from "node:path"; import { describe, expect, it } from "vitest"; -import { makePathEnv, makeTempDir } from "./exec-approvals-test-helpers.js"; -import { - analyzeArgvCommand, - analyzeShellCommand, - buildEnforcedShellCommand, - buildSafeBinsShellCommand, - evaluateExecAllowlist, - evaluateShellAllowlist, - normalizeSafeBins, - type ExecAllowlistEntry, -} from "./exec-approvals.js"; - -describe("exec approvals safe shell command builder", () => { - it("quotes only safeBins segments (leaves other segments untouched)", () => { - if (process.platform === "win32") { - return; - } - - const analysis = analyzeShellCommand({ - command: "rg foo src/*.ts | head -n 5 && echo ok", - cwd: "/tmp", - env: { PATH: "/usr/bin:/bin" }, - platform: process.platform, - }); - expect(analysis.ok).toBe(true); - - const res = buildSafeBinsShellCommand({ - command: "rg foo src/*.ts | head -n 5 && echo ok", - segments: analysis.segments, - segmentSatisfiedBy: [null, "safeBins", null], - platform: process.platform, - }); - expect(res.ok).toBe(true); - // Preserve non-safeBins segment raw (glob stays unquoted) - expect(res.command).toContain("rg foo src/*.ts"); - // SafeBins segment is fully quoted and pinned to its resolved absolute path. - expect(res.command).toMatch(/'[^']*\/head' '-n' '5'/); - }); - - it("enforces canonical planned argv for every approved segment", () => { - if (process.platform === "win32") { - return; - } - const analysis = analyzeShellCommand({ - command: "env rg -n needle", - cwd: "/tmp", - env: { PATH: "/usr/bin:/bin" }, - platform: process.platform, - }); - expect(analysis.ok).toBe(true); - const res = buildEnforcedShellCommand({ - command: "env rg -n needle", - segments: analysis.segments, - platform: process.platform, - }); - expect(res.ok).toBe(true); - expect(res.command).toMatch(/'(?:[^']*\/)?rg' '-n' 'needle'/); - expect(res.command).not.toContain("'env'"); - }); -}); - -describe("exec approvals shell parsing", () => { - it("parses pipelines and chained commands", () => { - const cases = [ - { - name: "pipeline", - command: "echo ok | jq .foo", - expectedSegments: ["echo", "jq"], - }, - { - name: "chain", - command: "ls && rm -rf /", - expectedChainHeads: ["ls", "rm"], - }, - ] as const; - for (const testCase of cases) { - const res = analyzeShellCommand({ command: testCase.command }); - expect(res.ok, testCase.name).toBe(true); - if ("expectedSegments" in testCase) { - expect( - res.segments.map((seg) => seg.argv[0]), - testCase.name, - ).toEqual(testCase.expectedSegments); - } else { - expect( - res.chains?.map((chain) => chain[0]?.argv[0]), - testCase.name, - ).toEqual(testCase.expectedChainHeads); - } - } - }); - - it("parses argv commands", () => { - const res = analyzeArgvCommand({ argv: ["/bin/echo", "ok"] }); - expect(res.ok).toBe(true); - expect(res.segments[0]?.argv).toEqual(["/bin/echo", "ok"]); - }); - - it("rejects unsupported shell constructs", () => { - const cases: Array<{ command: string; reason: string; platform?: NodeJS.Platform }> = [ - { command: 'echo "output: $(whoami)"', reason: "unsupported shell token: $()" }, - { command: 'echo "output: `id`"', reason: "unsupported shell token: `" }, - { command: "echo $(whoami)", reason: "unsupported shell token: $()" }, - { command: "cat < input.txt", reason: "unsupported shell token: <" }, - { command: "echo ok > output.txt", reason: "unsupported shell token: >" }, - { - command: "/usr/bin/echo first line\n/usr/bin/echo second line", - reason: "unsupported shell token: \n", - }, - { - command: 'echo "ok $\\\n(id -u)"', - reason: "unsupported shell token: newline", - }, - { - command: 'echo "ok $\\\r\n(id -u)"', - reason: "unsupported shell token: newline", - }, - { - command: "ping 127.0.0.1 -n 1 & whoami", - reason: "unsupported windows shell token: &", - platform: "win32", - }, - ]; - for (const testCase of cases) { - const res = analyzeShellCommand({ command: testCase.command, platform: testCase.platform }); - expect(res.ok).toBe(false); - expect(res.reason).toBe(testCase.reason); - } - }); - - it("accepts inert substitution-like syntax", () => { - const cases = ['echo "output: \\$(whoami)"', "echo 'output: $(whoami)'"]; - for (const command of cases) { - const res = analyzeShellCommand({ command }); - expect(res.ok).toBe(true); - expect(res.segments[0]?.argv[0]).toBe("echo"); - } - }); - - it("accepts safe heredoc forms", () => { - const cases: Array<{ command: string; expectedArgv: string[] }> = [ - { command: "/usr/bin/tee /tmp/file << 'EOF'\nEOF", expectedArgv: ["/usr/bin/tee"] }, - { command: "/usr/bin/tee /tmp/file < segment.argv[0])).toEqual(testCase.expectedArgv); - } - }); - - it("rejects unsafe or malformed heredoc forms", () => { - const cases: Array<{ command: string; reason: string }> = [ - { - command: "/usr/bin/cat < { - const res = analyzeShellCommand({ - command: '"C:\\Program Files\\Tool\\tool.exe" --version', - platform: "win32", - }); - expect(res.ok).toBe(true); - expect(res.segments[0]?.argv).toEqual(["C:\\Program Files\\Tool\\tool.exe", "--version"]); - }); -}); - -describe("exec approvals shell allowlist (chained commands)", () => { - it("evaluates chained command allowlist scenarios", () => { - const cases: Array<{ - allowlist: ExecAllowlistEntry[]; - command: string; - expectedAnalysisOk: boolean; - expectedAllowlistSatisfied: boolean; - platform?: NodeJS.Platform; - }> = [ - { - allowlist: [{ pattern: "/usr/bin/obsidian-cli" }, { pattern: "/usr/bin/head" }], - command: - "/usr/bin/obsidian-cli print-default && /usr/bin/obsidian-cli search foo | /usr/bin/head", - expectedAnalysisOk: true, - expectedAllowlistSatisfied: true, - }, - { - allowlist: [{ pattern: "/usr/bin/obsidian-cli" }], - command: "/usr/bin/obsidian-cli print-default && /usr/bin/rm -rf /", - expectedAnalysisOk: true, - expectedAllowlistSatisfied: false, - }, - { - allowlist: [{ pattern: "/usr/bin/echo" }], - command: "/usr/bin/echo ok &&", - expectedAnalysisOk: false, - expectedAllowlistSatisfied: false, - }, - { - allowlist: [{ pattern: "/usr/bin/ping" }], - command: "ping 127.0.0.1 -n 1 & whoami", - expectedAnalysisOk: false, - expectedAllowlistSatisfied: false, - platform: "win32", - }, - ]; - for (const testCase of cases) { - const result = evaluateShellAllowlist({ - command: testCase.command, - allowlist: testCase.allowlist, - safeBins: new Set(), - cwd: "/tmp", - platform: testCase.platform, - }); - expect(result.analysisOk).toBe(testCase.expectedAnalysisOk); - expect(result.allowlistSatisfied).toBe(testCase.expectedAllowlistSatisfied); - } - }); - - it("respects quoted chain separators", () => { - const allowlist: ExecAllowlistEntry[] = [{ pattern: "/usr/bin/echo" }]; - const commands = ['/usr/bin/echo "foo && bar"', '/usr/bin/echo "foo\\" && bar"']; - for (const command of commands) { - const result = evaluateShellAllowlist({ - command, - allowlist, - safeBins: new Set(), - cwd: "/tmp", - }); - expect(result.analysisOk).toBe(true); - expect(result.allowlistSatisfied).toBe(true); - } - }); - - it("fails allowlist analysis for shell line continuations", () => { - const result = evaluateShellAllowlist({ - command: 'echo "ok $\\\n(id -u)"', - allowlist: [{ pattern: "/usr/bin/echo" }], - safeBins: new Set(), - cwd: "/tmp", - }); - expect(result.analysisOk).toBe(false); - expect(result.allowlistSatisfied).toBe(false); - }); - - it("satisfies allowlist when bare * wildcard is present", () => { - const dir = makeTempDir(); - const binPath = path.join(dir, "mybin"); - fs.writeFileSync(binPath, "#!/bin/sh\n", { mode: 0o755 }); - const env = makePathEnv(dir); - try { - const result = evaluateShellAllowlist({ - command: "mybin --flag", - allowlist: [{ pattern: "*" }], - safeBins: new Set(), - cwd: dir, - env, - }); - expect(result.analysisOk).toBe(true); - expect(result.allowlistSatisfied).toBe(true); - } finally { - fs.rmSync(dir, { recursive: true, force: true }); - } - }); -}); +import { normalizeSafeBins } from "./exec-approvals-allowlist.js"; +import { evaluateExecAllowlist, type ExecAllowlistEntry } from "./exec-approvals.js"; describe("exec approvals allowlist evaluation", () => { function evaluateAutoAllowSkills(params: { From 3e6c8376fb636076b5b82331b4406c1216833ef6 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:19:15 +0000 Subject: [PATCH 05/12] test: tighten outbound send service coverage --- .../outbound/outbound-send-service.test.ts | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) diff --git a/src/infra/outbound/outbound-send-service.test.ts b/src/infra/outbound/outbound-send-service.test.ts index 68c956d93fc..391abee8dda 100644 --- a/src/infra/outbound/outbound-send-service.test.ts +++ b/src/infra/outbound/outbound-send-service.test.ts @@ -156,6 +156,78 @@ describe("executeSendAction", () => { ); }); + it("falls back to message and media params for plugin-handled mirror writes", async () => { + mocks.dispatchChannelMessageAction.mockResolvedValue(pluginActionResult("msg-plugin")); + + await executeSendAction({ + ctx: { + cfg: {}, + channel: "discord", + params: { to: "channel:123", message: "hello" }, + dryRun: false, + mirror: { + sessionKey: "agent:main:discord:channel:123", + agentId: "agent-9", + }, + }, + to: "channel:123", + message: "hello", + mediaUrls: ["https://example.com/a.png", "https://example.com/b.png"], + }); + + expect(mocks.appendAssistantMessageToSessionTranscript).toHaveBeenCalledWith( + expect.objectContaining({ + agentId: "agent-9", + sessionKey: "agent:main:discord:channel:123", + text: "hello", + mediaUrls: ["https://example.com/a.png", "https://example.com/b.png"], + }), + ); + }); + + it("skips plugin dispatch during dry-run sends and forwards gateway + silent to sendMessage", async () => { + mocks.sendMessage.mockResolvedValue({ + channel: "discord", + to: "channel:123", + via: "gateway", + mediaUrl: null, + }); + + await executeSendAction({ + ctx: { + cfg: {}, + channel: "discord", + params: { to: "channel:123", message: "hello" }, + dryRun: true, + silent: true, + gateway: { + url: "http://127.0.0.1:18789", + token: "tok", + timeoutMs: 5000, + clientName: "gateway", + mode: "gateway", + }, + }, + to: "channel:123", + message: "hello", + }); + + expect(mocks.dispatchChannelMessageAction).not.toHaveBeenCalled(); + expect(mocks.sendMessage).toHaveBeenCalledWith( + expect.objectContaining({ + to: "channel:123", + content: "hello", + dryRun: true, + silent: true, + gateway: expect.objectContaining({ + url: "http://127.0.0.1:18789", + token: "tok", + timeoutMs: 5000, + }), + }), + ); + }); + it("forwards poll args to sendPoll on core outbound path", async () => { mocks.dispatchChannelMessageAction.mockResolvedValue(null); mocks.sendPoll.mockResolvedValue({ @@ -200,4 +272,55 @@ describe("executeSendAction", () => { }), ); }); + + it("skips plugin dispatch during dry-run polls and forwards durationHours + silent", async () => { + mocks.sendPoll.mockResolvedValue({ + channel: "discord", + to: "channel:123", + question: "Lunch?", + options: ["Pizza", "Sushi"], + maxSelections: 1, + durationSeconds: null, + durationHours: 6, + via: "gateway", + }); + + await executePollAction({ + ctx: { + cfg: {}, + channel: "discord", + params: {}, + dryRun: true, + silent: true, + gateway: { + url: "http://127.0.0.1:18789", + token: "tok", + timeoutMs: 5000, + clientName: "gateway", + mode: "gateway", + }, + }, + to: "channel:123", + question: "Lunch?", + options: ["Pizza", "Sushi"], + maxSelections: 1, + durationHours: 6, + }); + + expect(mocks.dispatchChannelMessageAction).not.toHaveBeenCalled(); + expect(mocks.sendPoll).toHaveBeenCalledWith( + expect.objectContaining({ + to: "channel:123", + question: "Lunch?", + durationHours: 6, + dryRun: true, + silent: true, + gateway: expect.objectContaining({ + url: "http://127.0.0.1:18789", + token: "tok", + timeoutMs: 5000, + }), + }), + ); + }); }); From 28b0d8e8bd1c146684d6b7b7e4b0cc74af0d2a44 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Fri, 13 Mar 2026 17:19:40 -0400 Subject: [PATCH 06/12] fix(cron): prevent isolated cron nested lane deadlocks (#45459) * fix(cron): resolve isolated session deadlock (#44805) Map cron lane to nested in resolveGlobalLane to prevent deadlock when isolated cron jobs trigger inner operations (e.g. compaction). Outer execution holds the cron lane slot; inner work now uses nested lane. Co-Authored-By: Claude Opus 4.6 * docs(changelog): add cron isolated deadlock note --------- Co-authored-by: zhujian Co-authored-by: Claude Opus 4.6 --- CHANGELOG.md | 1 + src/agents/pi-embedded-runner/lanes.test.ts | 44 +++++++++++++++++++++ src/agents/pi-embedded-runner/lanes.ts | 4 ++ 3 files changed, 49 insertions(+) create mode 100644 src/agents/pi-embedded-runner/lanes.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 54a67b8a5a8..0e61358e91e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ Docs: https://docs.openclaw.ai - Browser/existing-session: accept text-only `list_pages` and `new_page` responses from Chrome DevTools MCP so live-session tab discovery and new-tab open flows keep working when the server omits structured page metadata. - Ollama/reasoning visibility: stop promoting native `thinking` and `reasoning` fields into final assistant text so local reasoning models no longer leak internal thoughts in normal replies. (#45330) Thanks @xi7ang. +- Cron/isolated sessions: route nested cron-triggered embedded runner work onto the nested lane so isolated cron jobs no longer deadlock when compaction or other queued inner work runs. Thanks @vincentkoc. - Windows/gateway install: bound `schtasks` calls and fall back to the Startup-folder login item when task creation hangs, so native `openclaw gateway install` fails fast instead of wedging forever on broken Scheduled Task setups. - Windows/gateway auth: stop attaching device identity on local loopback shared-token and password gateway calls, so native Windows agent replies no longer log stale `device signature expired` fallback noise before succeeding. - Telegram/media downloads: thread the same direct or proxy transport policy into SSRF-guarded file fetches so inbound attachments keep working when Telegram falls back between env-proxy and direct networking. (#44639) Thanks @obviyus. diff --git a/src/agents/pi-embedded-runner/lanes.test.ts b/src/agents/pi-embedded-runner/lanes.test.ts new file mode 100644 index 00000000000..f3625ddc6ec --- /dev/null +++ b/src/agents/pi-embedded-runner/lanes.test.ts @@ -0,0 +1,44 @@ +import { describe, expect, it } from "vitest"; +import { CommandLane } from "../../process/lanes.js"; +import { resolveGlobalLane, resolveSessionLane } from "./lanes.js"; + +describe("resolveGlobalLane", () => { + it("defaults to main lane when no lane is provided", () => { + expect(resolveGlobalLane()).toBe(CommandLane.Main); + expect(resolveGlobalLane("")).toBe(CommandLane.Main); + expect(resolveGlobalLane(" ")).toBe(CommandLane.Main); + }); + + it("maps cron lane to nested lane to prevent deadlocks", () => { + // When cron jobs trigger nested agent runs, the outer execution holds + // the cron lane slot. Inner work must use a separate lane to avoid + // deadlock. See: https://github.com/openclaw/openclaw/issues/44805 + expect(resolveGlobalLane("cron")).toBe(CommandLane.Nested); + expect(resolveGlobalLane(" cron ")).toBe(CommandLane.Nested); + }); + + it("preserves other lanes as-is", () => { + expect(resolveGlobalLane("main")).toBe(CommandLane.Main); + expect(resolveGlobalLane("subagent")).toBe(CommandLane.Subagent); + expect(resolveGlobalLane("nested")).toBe(CommandLane.Nested); + expect(resolveGlobalLane("custom-lane")).toBe("custom-lane"); + expect(resolveGlobalLane(" custom ")).toBe("custom"); + }); +}); + +describe("resolveSessionLane", () => { + it("defaults to main lane and prefixes with session:", () => { + expect(resolveSessionLane("")).toBe("session:main"); + expect(resolveSessionLane(" ")).toBe("session:main"); + }); + + it("adds session: prefix if not present", () => { + expect(resolveSessionLane("abc123")).toBe("session:abc123"); + expect(resolveSessionLane(" xyz ")).toBe("session:xyz"); + }); + + it("preserves existing session: prefix", () => { + expect(resolveSessionLane("session:abc")).toBe("session:abc"); + expect(resolveSessionLane("session:main")).toBe("session:main"); + }); +}); diff --git a/src/agents/pi-embedded-runner/lanes.ts b/src/agents/pi-embedded-runner/lanes.ts index 81b742ded9f..57ffd1b4255 100644 --- a/src/agents/pi-embedded-runner/lanes.ts +++ b/src/agents/pi-embedded-runner/lanes.ts @@ -7,6 +7,10 @@ export function resolveSessionLane(key: string) { export function resolveGlobalLane(lane?: string) { const cleaned = lane?.trim(); + // Cron jobs hold the cron lane slot; inner operations must use nested to avoid deadlock. + if (cleaned === CommandLane.Cron) { + return CommandLane.Nested; + } return cleaned ? cleaned : CommandLane.Main; } From 651ccf99015487689da3d76adf1ebc97004ac945 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:20:20 +0000 Subject: [PATCH 07/12] test: tighten channel selection coverage --- src/infra/outbound/channel-selection.test.ts | 110 +++++++++++++++++-- 1 file changed, 101 insertions(+), 9 deletions(-) diff --git a/src/infra/outbound/channel-selection.test.ts b/src/infra/outbound/channel-selection.test.ts index 15642a33bb1..da605dcdb63 100644 --- a/src/infra/outbound/channel-selection.test.ts +++ b/src/infra/outbound/channel-selection.test.ts @@ -8,7 +8,83 @@ vi.mock("../../channels/plugins/index.js", () => ({ listChannelPlugins: mocks.listChannelPlugins, })); -import { resolveMessageChannelSelection } from "./channel-selection.js"; +import { + listConfiguredMessageChannels, + resolveMessageChannelSelection, +} from "./channel-selection.js"; + +function makePlugin(params: { + id: string; + accountIds?: string[]; + resolveAccount?: (accountId: string) => unknown; + isEnabled?: (account: unknown) => boolean; + isConfigured?: (account: unknown) => boolean | Promise; +}) { + return { + id: params.id, + config: { + listAccountIds: () => params.accountIds ?? ["default"], + resolveAccount: (_cfg: unknown, accountId: string) => + params.resolveAccount ? params.resolveAccount(accountId) : {}, + ...(params.isEnabled ? { isEnabled: params.isEnabled } : {}), + ...(params.isConfigured ? { isConfigured: params.isConfigured } : {}), + }, + }; +} + +describe("listConfiguredMessageChannels", () => { + beforeEach(() => { + mocks.listChannelPlugins.mockReset(); + mocks.listChannelPlugins.mockReturnValue([]); + }); + + it("skips unknown plugin ids and plugins without accounts", async () => { + mocks.listChannelPlugins.mockReturnValue([ + makePlugin({ id: "not-a-channel" }), + makePlugin({ id: "slack", accountIds: [] }), + ]); + + await expect(listConfiguredMessageChannels({} as never)).resolves.toEqual([]); + }); + + it("includes plugins without isConfigured when an enabled account exists", async () => { + mocks.listChannelPlugins.mockReturnValue([ + makePlugin({ + id: "discord", + resolveAccount: () => ({ enabled: true }), + }), + ]); + + await expect(listConfiguredMessageChannels({} as never)).resolves.toEqual(["discord"]); + }); + + it("skips disabled accounts and keeps later configured accounts", async () => { + mocks.listChannelPlugins.mockReturnValue([ + makePlugin({ + id: "telegram", + accountIds: ["disabled", "enabled"], + resolveAccount: (accountId) => + accountId === "disabled" ? { enabled: false } : { enabled: true }, + isConfigured: (account) => (account as { enabled?: boolean }).enabled === true, + }), + ]); + + await expect(listConfiguredMessageChannels({} as never)).resolves.toEqual(["telegram"]); + }); + + it("respects custom isEnabled checks", async () => { + mocks.listChannelPlugins.mockReturnValue([ + makePlugin({ + id: "signal", + resolveAccount: () => ({ token: "x" }), + isEnabled: () => false, + isConfigured: () => true, + }), + ]); + + await expect(listConfiguredMessageChannels({} as never)).resolves.toEqual([]); + }); +}); describe("resolveMessageChannelSelection", () => { beforeEach(() => { @@ -58,14 +134,7 @@ describe("resolveMessageChannelSelection", () => { it("selects single configured channel when no explicit/fallback channel exists", async () => { mocks.listChannelPlugins.mockReturnValue([ - { - id: "discord", - config: { - listAccountIds: () => ["default"], - resolveAccount: () => ({}), - isConfigured: async () => true, - }, - }, + makePlugin({ id: "discord", isConfigured: async () => true }), ]); const selection = await resolveMessageChannelSelection({ @@ -88,4 +157,27 @@ describe("resolveMessageChannelSelection", () => { }), ).rejects.toThrow("Unknown channel: channel:c123"); }); + + it("throws when no channel is provided and nothing is configured", async () => { + await expect( + resolveMessageChannelSelection({ + cfg: {} as never, + }), + ).rejects.toThrow("Channel is required (no configured channels detected)."); + }); + + it("throws when multiple channels are configured and no channel is selected", async () => { + mocks.listChannelPlugins.mockReturnValue([ + makePlugin({ id: "discord", isConfigured: async () => true }), + makePlugin({ id: "telegram", isConfigured: async () => true }), + ]); + + await expect( + resolveMessageChannelSelection({ + cfg: {} as never, + }), + ).rejects.toThrow( + "Channel is required when multiple channels are configured: discord, telegram", + ); + }); }); From 256c91ca6d4a0db2d331d197a4b96a382cf78339 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:22:15 +0000 Subject: [PATCH 08/12] test: tighten message action normalization coverage --- .../message-action-normalization.test.ts | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/src/infra/outbound/message-action-normalization.test.ts b/src/infra/outbound/message-action-normalization.test.ts index 5f0647b955c..87fa7a8503c 100644 --- a/src/infra/outbound/message-action-normalization.test.ts +++ b/src/infra/outbound/message-action-normalization.test.ts @@ -72,6 +72,62 @@ describe("normalizeMessageActionInput", () => { expect(normalized.channel).toBe("slack"); }); + it("does not infer a target for actions that do not accept one", () => { + const normalized = normalizeMessageActionInput({ + action: "broadcast", + args: {}, + toolContext: { + currentChannelId: "channel:C1", + }, + }); + + expect("target" in normalized).toBe(false); + expect("to" in normalized).toBe(false); + }); + + it("does not backfill a non-deliverable tool-context channel", () => { + const normalized = normalizeMessageActionInput({ + action: "send", + args: { + target: "channel:C1", + }, + toolContext: { + currentChannelProvider: "webchat", + }, + }); + + expect("channel" in normalized).toBe(false); + }); + + it("keeps alias-based targets without inferring the current channel", () => { + const normalized = normalizeMessageActionInput({ + action: "edit", + args: { + messageId: "msg_123", + }, + toolContext: { + currentChannelId: "channel:C1", + }, + }); + + expect(normalized.messageId).toBe("msg_123"); + expect("target" in normalized).toBe(false); + expect("to" in normalized).toBe(false); + }); + + it("maps legacy channelId inputs through canonical target for channel-id actions", () => { + const normalized = normalizeMessageActionInput({ + action: "channel-info", + args: { + channelId: "C123", + }, + }); + + expect(normalized.target).toBe("C123"); + expect(normalized.channelId).toBe("C123"); + expect("to" in normalized).toBe(false); + }); + it("throws when required target remains unresolved", () => { expect(() => normalizeMessageActionInput({ From e8addf2ac22f97a275aa48b66be04e6d98310da7 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:23:10 +0000 Subject: [PATCH 09/12] test: add message action spec coverage --- .../outbound/message-action-spec.test.ts | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 src/infra/outbound/message-action-spec.test.ts diff --git a/src/infra/outbound/message-action-spec.test.ts b/src/infra/outbound/message-action-spec.test.ts new file mode 100644 index 00000000000..138f61e08a0 --- /dev/null +++ b/src/infra/outbound/message-action-spec.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it } from "vitest"; +import { actionHasTarget, actionRequiresTarget } from "./message-action-spec.js"; + +describe("actionRequiresTarget", () => { + it.each([ + ["send", true], + ["channel-info", true], + ["broadcast", false], + ["search", false], + ])("returns %s for %s", (action, expected) => { + expect(actionRequiresTarget(action as never)).toBe(expected); + }); +}); + +describe("actionHasTarget", () => { + it("detects canonical target fields", () => { + expect(actionHasTarget("send", { to: " channel:C1 " })).toBe(true); + expect(actionHasTarget("channel-info", { channelId: " C123 " })).toBe(true); + expect(actionHasTarget("send", { to: " ", channelId: "" })).toBe(false); + }); + + it("detects alias targets for message and chat actions", () => { + expect(actionHasTarget("edit", { messageId: " msg_123 " })).toBe(true); + expect(actionHasTarget("react", { chatGuid: "chat-guid" })).toBe(true); + expect(actionHasTarget("react", { chatIdentifier: "chat-id" })).toBe(true); + expect(actionHasTarget("react", { chatId: 42 })).toBe(true); + }); + + it("rejects blank and non-finite alias targets", () => { + expect(actionHasTarget("edit", { messageId: " " })).toBe(false); + expect(actionHasTarget("react", { chatGuid: "" })).toBe(false); + expect(actionHasTarget("react", { chatId: Number.NaN })).toBe(false); + expect(actionHasTarget("react", { chatId: Number.POSITIVE_INFINITY })).toBe(false); + }); + + it("ignores alias fields for actions without alias target support", () => { + expect(actionHasTarget("send", { messageId: "msg_123", chatId: 42 })).toBe(false); + }); +}); From 42ccee658d738646053142641c7e8e5dbaa78cdb Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:24:38 +0000 Subject: [PATCH 10/12] test: tighten shared avatar and scope coverage --- src/shared/avatar-policy.test.ts | 19 +++++++++++++++++ src/shared/operator-scope-compat.test.ts | 27 ++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/src/shared/avatar-policy.test.ts b/src/shared/avatar-policy.test.ts index 81331a45b8d..cbc345767e7 100644 --- a/src/shared/avatar-policy.test.ts +++ b/src/shared/avatar-policy.test.ts @@ -1,24 +1,42 @@ import path from "node:path"; import { describe, expect, it } from "vitest"; import { + hasAvatarUriScheme, + isAvatarDataUrl, + isAvatarHttpUrl, + isAvatarImageDataUrl, isPathWithinRoot, isSupportedLocalAvatarExtension, + isWindowsAbsolutePath, isWorkspaceRelativeAvatarPath, looksLikeAvatarPath, resolveAvatarMime, } from "./avatar-policy.js"; describe("avatar policy", () => { + it("classifies avatar URI and path helpers directly", () => { + expect(isAvatarDataUrl("data:text/plain,hello")).toBe(true); + expect(isAvatarImageDataUrl("data:image/png;base64,AAAA")).toBe(true); + expect(isAvatarImageDataUrl("data:text/plain,hello")).toBe(false); + expect(isAvatarHttpUrl("https://example.com/avatar.png")).toBe(true); + expect(isAvatarHttpUrl("ftp://example.com/avatar.png")).toBe(false); + expect(hasAvatarUriScheme("slack://avatar")).toBe(true); + expect(isWindowsAbsolutePath("C:\\\\avatars\\\\openclaw.png")).toBe(true); + }); + it("accepts workspace-relative avatar paths and rejects URI schemes", () => { expect(isWorkspaceRelativeAvatarPath("avatars/openclaw.png")).toBe(true); expect(isWorkspaceRelativeAvatarPath("C:\\\\avatars\\\\openclaw.png")).toBe(true); expect(isWorkspaceRelativeAvatarPath("https://example.com/avatar.png")).toBe(false); expect(isWorkspaceRelativeAvatarPath("data:image/png;base64,AAAA")).toBe(false); expect(isWorkspaceRelativeAvatarPath("~/avatar.png")).toBe(false); + expect(isWorkspaceRelativeAvatarPath("slack://avatar")).toBe(false); + expect(isWorkspaceRelativeAvatarPath("")).toBe(false); }); it("checks path containment safely", () => { const root = path.resolve("/tmp/root"); + expect(isPathWithinRoot(root, root)).toBe(true); expect(isPathWithinRoot(root, path.resolve("/tmp/root/avatars/a.png"))).toBe(true); expect(isPathWithinRoot(root, path.resolve("/tmp/root/../outside.png"))).toBe(false); }); @@ -38,6 +56,7 @@ describe("avatar policy", () => { it("resolves mime type from extension", () => { expect(resolveAvatarMime("a.svg")).toBe("image/svg+xml"); expect(resolveAvatarMime("a.tiff")).toBe("image/tiff"); + expect(resolveAvatarMime("A.PNG")).toBe("image/png"); expect(resolveAvatarMime("a.bin")).toBe("application/octet-stream"); }); }); diff --git a/src/shared/operator-scope-compat.test.ts b/src/shared/operator-scope-compat.test.ts index 11810673681..e48a17ad398 100644 --- a/src/shared/operator-scope-compat.test.ts +++ b/src/shared/operator-scope-compat.test.ts @@ -86,4 +86,31 @@ describe("roleScopesAllow", () => { }), ).toBe(false); }); + + it("normalizes blank and duplicate scopes before evaluating", () => { + expect( + roleScopesAllow({ + role: " operator ", + requestedScopes: [" operator.read ", "operator.read", " "], + allowedScopes: [" operator.write ", "operator.write", ""], + }), + ).toBe(true); + }); + + it("rejects unsatisfied operator write scopes and empty allowed scopes", () => { + expect( + roleScopesAllow({ + role: "operator", + requestedScopes: ["operator.write"], + allowedScopes: ["operator.read"], + }), + ).toBe(false); + expect( + roleScopesAllow({ + role: "operator", + requestedScopes: ["operator.read"], + allowedScopes: [" "], + }), + ).toBe(false); + }); }); From 0c79c86b40a02338891365d6ecce9bc004fdef01 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:25:20 +0000 Subject: [PATCH 11/12] test: tighten shared singleton and sample coverage --- src/shared/global-singleton.test.ts | 16 ++++++++++++++++ src/shared/string-sample.test.ts | 24 ++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/src/shared/global-singleton.test.ts b/src/shared/global-singleton.test.ts index 0f0a29c506c..3d537f5cc4b 100644 --- a/src/shared/global-singleton.test.ts +++ b/src/shared/global-singleton.test.ts @@ -27,6 +27,15 @@ describe("resolveGlobalSingleton", () => { expect(resolveGlobalSingleton(TEST_KEY, create)).toBeUndefined(); expect(create).toHaveBeenCalledTimes(1); }); + + it("reuses a prepopulated global value without calling the factory", () => { + const existing = { value: 7 }; + const create = vi.fn(() => ({ value: 1 })); + (globalThis as Record)[TEST_KEY] = existing; + + expect(resolveGlobalSingleton(TEST_KEY, create)).toBe(existing); + expect(create).not.toHaveBeenCalled(); + }); }); describe("resolveGlobalMap", () => { @@ -36,4 +45,11 @@ describe("resolveGlobalMap", () => { expect(first).toBe(second); }); + + it("preserves existing map contents across repeated resolution", () => { + const map = resolveGlobalMap(TEST_MAP_KEY); + map.set("a", 1); + + expect(resolveGlobalMap(TEST_MAP_KEY).get("a")).toBe(1); + }); }); diff --git a/src/shared/string-sample.test.ts b/src/shared/string-sample.test.ts index 4cff7957fe0..7ced1e7407a 100644 --- a/src/shared/string-sample.test.ts +++ b/src/shared/string-sample.test.ts @@ -4,6 +4,7 @@ import { summarizeStringEntries } from "./string-sample.js"; describe("summarizeStringEntries", () => { it("returns emptyText for empty lists", () => { expect(summarizeStringEntries({ entries: [], emptyText: "any" })).toBe("any"); + expect(summarizeStringEntries({ entries: null })).toBe(""); }); it("joins short lists without a suffix", () => { @@ -18,4 +19,27 @@ describe("summarizeStringEntries", () => { }), ).toBe("a, b, c, d (+1)"); }); + + it("uses a floored limit and clamps non-positive values to one entry", () => { + expect( + summarizeStringEntries({ + entries: ["a", "b", "c"], + limit: 2.8, + }), + ).toBe("a, b (+1)"); + expect( + summarizeStringEntries({ + entries: ["a", "b", "c"], + limit: 0, + }), + ).toBe("a (+2)"); + }); + + it("uses the default limit when none is provided", () => { + expect( + summarizeStringEntries({ + entries: ["a", "b", "c", "d", "e", "f", "g"], + }), + ).toBe("a, b, c, d, e, f (+1)"); + }); }); From 090c0c4b5d3db025f07478168f3348894cb5ac1b Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 21:26:15 +0000 Subject: [PATCH 12/12] test: tighten shared text normalization coverage --- src/shared/string-normalization.test.ts | 10 ++++++++++ src/shared/text/join-segments.test.ts | 13 +++++++++++++ 2 files changed, 23 insertions(+) diff --git a/src/shared/string-normalization.test.ts b/src/shared/string-normalization.test.ts index ca92a8ae89c..e0f8c8ae900 100644 --- a/src/shared/string-normalization.test.ts +++ b/src/shared/string-normalization.test.ts @@ -29,10 +29,20 @@ describe("shared/string-normalization", () => { expect(normalizeHyphenSlug(null)).toBe(""); }); + it("collapses repeated separators and trims leading/trailing punctuation", () => { + expect(normalizeHyphenSlug(" ...Hello / World--- ")).toBe("hello-world"); + expect(normalizeHyphenSlug(" ###Team@@@Room### ")).toBe("###team@@@room###"); + }); + it("normalizes @/# prefixed slugs used by channel allowlists", () => { expect(normalizeAtHashSlug(" #My_Channel + Alerts ")).toBe("my-channel-alerts"); expect(normalizeAtHashSlug("@@Room___Name")).toBe("room-name"); expect(normalizeAtHashSlug(undefined)).toBe(""); expect(normalizeAtHashSlug(null)).toBe(""); }); + + it("strips repeated prefixes and collapses separator-only results", () => { + expect(normalizeAtHashSlug("###__Room Name__")).toBe("room-name"); + expect(normalizeAtHashSlug("@@@___")).toBe(""); + }); }); diff --git a/src/shared/text/join-segments.test.ts b/src/shared/text/join-segments.test.ts index 279516e4269..8da5c4644a7 100644 --- a/src/shared/text/join-segments.test.ts +++ b/src/shared/text/join-segments.test.ts @@ -9,6 +9,12 @@ describe("concatOptionalTextSegments", () => { it("keeps explicit empty-string right value", () => { expect(concatOptionalTextSegments({ left: "A", right: "" })).toBe(""); }); + + it("falls back to whichever side is present and honors custom separators", () => { + expect(concatOptionalTextSegments({ left: "A" })).toBe("A"); + expect(concatOptionalTextSegments({ right: "B" })).toBe("B"); + expect(concatOptionalTextSegments({ left: "A", right: "B", separator: " | " })).toBe("A | B"); + }); }); describe("joinPresentTextSegments", () => { @@ -23,4 +29,11 @@ describe("joinPresentTextSegments", () => { it("trims segments when requested", () => { expect(joinPresentTextSegments([" A ", " B "], { trim: true })).toBe("A\n\nB"); }); + + it("keeps whitespace-only segments unless trim is enabled and supports custom separators", () => { + expect(joinPresentTextSegments(["A", " ", "B"], { separator: " | " })).toBe("A | | B"); + expect(joinPresentTextSegments(["A", " ", "B"], { trim: true, separator: " | " })).toBe( + "A | B", + ); + }); });