From 2a691495668c3df23a618252443632746b89c9d9 Mon Sep 17 00:00:00 2001 From: Masato Hoshino Date: Sat, 11 Jul 2026 09:52:37 +0900 Subject: [PATCH] fix(clickclack): sanitize outbound assistant text (#103142) --- extensions/clickclack/src/channel.ts | 15 +++++---- extensions/clickclack/src/outbound.test.ts | 37 +++++++++++++++------- extensions/clickclack/src/outbound.ts | 25 +++++++++------ 3 files changed, 51 insertions(+), 26 deletions(-) diff --git a/extensions/clickclack/src/channel.ts b/extensions/clickclack/src/channel.ts index 8d779801442..c07c692a2a6 100644 --- a/extensions/clickclack/src/channel.ts +++ b/extensions/clickclack/src/channel.ts @@ -49,7 +49,7 @@ const clickClackMessageAdapter = defineChannelMessageAdapter({ }, send: { text: async (ctx) => { - const result = await sendClickClackText({ + const messageId = await sendClickClackText({ cfg: ctx.cfg as CoreConfig, accountId: ctx.accountId, to: ctx.to, @@ -60,9 +60,9 @@ const clickClackMessageAdapter = defineChannelMessageAdapter({ const threadId = ctx.threadId == null ? undefined : String(ctx.threadId); const replyToId = ctx.replyToId ?? undefined; return { - messageId: result.messageId, + ...(messageId ? { messageId } : {}), receipt: createMessageReceiptFromOutboundResults({ - results: [{ channel: CHANNEL_ID, messageId: result.messageId }], + results: messageId ? [{ channel: CHANNEL_ID, messageId }] : [], threadId, replyToId, kind: "text", @@ -177,15 +177,18 @@ export const clickClackPlugin: ChannelPlugin = create }, attachedResults: { channel: CHANNEL_ID, - sendText: async ({ cfg, to, text, accountId, threadId, replyToId }) => - await sendClickClackText({ + sendText: async ({ cfg, to, text, accountId, threadId, replyToId }) => { + const messageId = await sendClickClackText({ cfg: cfg as CoreConfig, accountId, to, text, threadId, replyToId, - }), + }); + // Legacy outbound results use an empty id to report an intentional no-send. + return { messageId: messageId ?? "" }; + }, }, }, }); diff --git a/extensions/clickclack/src/outbound.test.ts b/extensions/clickclack/src/outbound.test.ts index 3a43c4bfff0..b9b151fefce 100644 --- a/extensions/clickclack/src/outbound.test.ts +++ b/extensions/clickclack/src/outbound.test.ts @@ -1,5 +1,4 @@ -// Covers outbound delivery routing: a reply to a top-level message must post to -// the main channel as a quote-reply, not open a per-reply thread. +// Covers ClickClack outbound routing and sender-boundary assistant text sanitization. import { beforeEach, describe, expect, it, vi } from "vitest"; import { sendClickClackText } from "./outbound.js"; import type { CoreConfig } from "./types.js"; @@ -46,18 +45,18 @@ describe("sendClickClackText routing", () => { createClientOptions.mockClear(); }); - it("delivers a reply to a top-level channel message as an in-channel quote-reply", async () => { + it("sanitizes a top-level channel quote-reply", async () => { await sendClickClackText({ cfg, to: "channel:general", - text: "hi", + text: "Done.\n⚠️ 🛠️ `search repos (agent)` failed", replyToId: "msg_root", }); expect(createChannelMessage).toHaveBeenCalledTimes(1); expect(createChannelMessage).toHaveBeenCalledWith( "general", - "hi", + "Done.", expect.objectContaining({ quotedMessageId: "msg_root" }), ); expect(createThreadReply).not.toHaveBeenCalled(); @@ -89,16 +88,16 @@ describe("sendClickClackText routing", () => { }); }); - it("keeps replies inside a genuine thread (explicit threadId)", async () => { + it("sanitizes replies inside a genuine thread", async () => { await sendClickClackText({ cfg, to: "channel:general", - text: "hi", + text: "Done.\n⚠️ 🛠️ `search repos (agent)` failed", threadId: "msg_thread_root", replyToId: "msg_root", }); - expect(createThreadReply).toHaveBeenCalledWith("msg_thread_root", "hi", expect.anything()); + expect(createThreadReply).toHaveBeenCalledWith("msg_thread_root", "Done.", expect.anything()); expect(createChannelMessage).not.toHaveBeenCalled(); }); @@ -109,19 +108,35 @@ describe("sendClickClackText routing", () => { expect(createChannelMessage).not.toHaveBeenCalled(); }); - it("delivers a DM reply as a quote-reply in the same conversation", async () => { + it("sanitizes leaked tool XML in a DM quote-reply", async () => { await sendClickClackText({ cfg, to: "dm:usr_1", - text: "hi", + text: '{"name":"exec"}Deploy finished.', replyToId: "msg_root", }); expect(createDirectMessage).toHaveBeenCalledWith( "dm_1", - "hi", + "Deploy finished.", expect.objectContaining({ quotedMessageId: "msg_root" }), ); expect(createThreadReply).not.toHaveBeenCalled(); }); + + it("suppresses replies containing only internal scaffolding", async () => { + await expect( + sendClickClackText({ + cfg, + to: "channel:general", + text: "⚠️ 🛠️ `search repos (agent)` failed", + }), + ).resolves.toBeUndefined(); + + expect(createClientOptions).not.toHaveBeenCalled(); + expect(createChannelMessage).not.toHaveBeenCalled(); + expect(createThreadReply).not.toHaveBeenCalled(); + expect(createDirectConversation).not.toHaveBeenCalled(); + expect(createDirectMessage).not.toHaveBeenCalled(); + }); }); diff --git a/extensions/clickclack/src/outbound.ts b/extensions/clickclack/src/outbound.ts index 8de6522d7ad..f27d09b28ca 100644 --- a/extensions/clickclack/src/outbound.ts +++ b/extensions/clickclack/src/outbound.ts @@ -2,6 +2,7 @@ * Outbound ClickClack delivery helpers for channel messages, thread replies, * and direct messages. */ +import { sanitizeAssistantVisibleText } from "openclaw/plugin-sdk/text-chunking"; import { resolveClickClackAccount } from "./accounts.js"; import { createClickClackClient } from "./http-client.js"; import { resolveChannelId, resolveWorkspaceId } from "./resolve.js"; @@ -9,8 +10,8 @@ import { parseClickClackTarget } from "./target.js"; import type { ClickClackMessageProvenance, CoreConfig } from "./types.js"; /** - * Sends text to a normalized ClickClack target and returns the created message - * id for receipt/session tracking. + * Sends visible text to a normalized ClickClack target and returns the created + * message id, or undefined when sanitization removes all content. */ export async function sendClickClackText(params: { cfg: CoreConfig; @@ -23,7 +24,13 @@ export async function sendClickClackText(params: { correlationId?: string; /** Optional model/thinking attribution stamped onto the created message. */ provenance?: ClickClackMessageProvenance; -}) { +}): Promise { + // Custom inbound replies bypass shared outbound normalization, so this private + // sender owns ClickClack assistant-text sanitization for every delivery path. + const text = sanitizeAssistantVisibleText(params.text); + if (!text) { + return undefined; + } const account = resolveClickClackAccount({ cfg: params.cfg, accountId: params.accountId }); const client = createClickClackClient({ baseUrl: account.baseUrl, @@ -41,25 +48,25 @@ export async function sendClickClackText(params: { // below — otherwise every channel reply spawns its own thread and the main // timeline goes silent. const rootId = explicitThreadId || parsed.id; - const message = await client.createThreadReply(rootId, params.text, { + const message = await client.createThreadReply(rootId, text, { provenance: params.provenance, }); - return { to: params.to, messageId: message.id }; + return message.id; } if (parsed.kind === "dm") { const dm = await client.createDirectConversation(workspaceId, [parsed.id]); - const message = await client.createDirectMessage(dm.id, params.text, { + const message = await client.createDirectMessage(dm.id, text, { quotedMessageId: replyToId || undefined, }); - return { to: params.to, messageId: message.id }; + return message.id; } const channelId = await resolveChannelId(client, workspaceId, parsed.id); // A reply to a top-level channel message is delivered to the main channel as a // quote-reply (quoted_message_id), matching the reply-to affordance of the // Discord/Slack/Telegram channels, instead of opening a per-reply thread. - const message = await client.createChannelMessage(channelId, params.text, { + const message = await client.createChannelMessage(channelId, text, { provenance: params.provenance, quotedMessageId: replyToId || undefined, }); - return { to: params.to, messageId: message.id }; + return message.id; }