fix(clickclack): sanitize outbound assistant text (#103142)

This commit is contained in:
Masato Hoshino
2026-07-10 17:52:37 -07:00
committed by GitHub
parent e44f04c50e
commit 2a69149566
3 changed files with 51 additions and 26 deletions
+9 -6
View File
@@ -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<ResolvedClickClackAccount> = 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 ?? "" };
},
},
},
});
+26 -11
View File
@@ -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: '<tool_call>{"name":"exec"}</tool_call>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();
});
});
+16 -9
View File
@@ -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<string | undefined> {
// 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;
}