Skip to content

Commit badf3f8

Browse files
fix(outbound): sanitize assistant reasoning before delivery
1 parent 6da73ac commit badf3f8

6 files changed

Lines changed: 269 additions & 6 deletions

File tree

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import { describe, expect, it, vi } from "vitest";
2+
import { createReplyDispatcher } from "./reply-dispatcher.js";
3+
4+
async function drain(): Promise<void> {
5+
await new Promise((resolve) => setTimeout(resolve, 0));
6+
}
7+
8+
describe("createReplyDispatcher delivery sanitizer", () => {
9+
it("sanitizes assistant block replies before custom delivery", async () => {
10+
const deliver = vi.fn(async () => undefined);
11+
const dispatcher = createReplyDispatcher({ deliver });
12+
13+
expect(dispatcher.sendBlockReply({ text: "<think>hidden</think>Visible" })).toBe(true);
14+
dispatcher.markComplete();
15+
await dispatcher.waitForIdle();
16+
17+
expect(deliver).toHaveBeenCalledWith({ text: "Visible" }, { kind: "block" });
18+
});
19+
20+
it("drops reasoning-only final replies before custom delivery", async () => {
21+
const deliver = vi.fn(async () => undefined);
22+
const dispatcher = createReplyDispatcher({ deliver });
23+
24+
expect(dispatcher.sendFinalReply({ text: "Reasoning:\n_private step_" })).toBe(false);
25+
dispatcher.markComplete();
26+
await drain();
27+
28+
expect(deliver).not.toHaveBeenCalled();
29+
});
30+
31+
it("does not sanitize tool result payloads", async () => {
32+
const deliver = vi.fn(async () => undefined);
33+
const dispatcher = createReplyDispatcher({ deliver });
34+
35+
expect(dispatcher.sendToolResult({ text: "Reasoning:\nliteral tool output" })).toBe(true);
36+
dispatcher.markComplete();
37+
await dispatcher.waitForIdle();
38+
39+
expect(deliver).toHaveBeenCalledWith(
40+
{ text: "Reasoning:\nliteral tool output" },
41+
{ kind: "tool" },
42+
);
43+
});
44+
});

src/auto-reply/reply/reply-dispatcher.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { TypingCallbacks } from "../../channels/typing.js";
22
import type { HumanDelayConfig } from "../../config/types.js";
33
import type { OpenClawConfig } from "../../config/types.openclaw.js";
4+
import { sanitizeAssistantForDelivery } from "../../infra/outbound/assistant-delivery-sanitizer.js";
45
import { generateSecureInt } from "../../infra/secure-random.js";
56
import { createSubsystemLogger } from "../../logging/subsystem.js";
67
import type { SilentReplyConversationType } from "../../shared/silent-reply-policy.js";
@@ -172,6 +173,13 @@ export function createReplyDispatcher(options: ReplyDispatcherOptions): ReplyDis
172173
}
173174
return false;
174175
}
176+
const deliverySanitized =
177+
kind === "tool"
178+
? normalized
179+
: sanitizeAssistantForDelivery(normalized, { role: "assistant" });
180+
if (!deliverySanitized) {
181+
return false;
182+
}
175183
queuedCounts[kind] += 1;
176184
pending += 1;
177185

@@ -190,9 +198,9 @@ export function createReplyDispatcher(options: ReplyDispatcherOptions): ReplyDis
190198
await sleep(delayMs);
191199
}
192200
}
193-
let deliverPayload: ReplyPayload | null = normalized;
201+
let deliverPayload: ReplyPayload | null = deliverySanitized;
194202
if (options.beforeDeliver) {
195-
deliverPayload = await options.beforeDeliver(normalized, { kind });
203+
deliverPayload = await options.beforeDeliver(deliverySanitized, { kind });
196204
if (!deliverPayload) {
197205
cancelledCounts[kind] += 1;
198206
return;
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import { describe, expect, it } from "vitest";
2+
import {
3+
sanitizeAssistantForDelivery,
4+
sanitizeAssistantTextForDelivery,
5+
} from "./assistant-delivery-sanitizer.js";
6+
7+
describe("sanitizeAssistantForDelivery", () => {
8+
it("strips assistant tagged thinking when thinking is off", () => {
9+
expect(sanitizeAssistantTextForDelivery("<think>hidden</think>Visible")).toBe("Visible");
10+
});
11+
12+
it("preserves assistant tagged thinking when thinking is enabled", () => {
13+
expect(
14+
sanitizeAssistantTextForDelivery("<think>hidden</think>Visible", {
15+
thinkingEnabled: true,
16+
}),
17+
).toBe("<think>hidden</think>Visible");
18+
});
19+
20+
it("does not sanitize non-assistant literal examples", () => {
21+
expect(
22+
sanitizeAssistantTextForDelivery("<think>example</think>", {
23+
role: "user",
24+
}),
25+
).toBe("<think>example</think>");
26+
});
27+
28+
it("drops reasoning-only outbound payloads", () => {
29+
expect(sanitizeAssistantForDelivery({ text: "Reasoning:\n_private step_" })).toBeNull();
30+
expect(sanitizeAssistantForDelivery({ text: "private step", isReasoning: true })).toBeNull();
31+
});
32+
33+
it("preserves blank text when payload metadata has channel content", () => {
34+
expect(sanitizeAssistantForDelivery({ text: " \n\t ", channelData: { mode: "flex" } })).toEqual(
35+
{
36+
text: " \n\t ",
37+
channelData: { mode: "flex" },
38+
},
39+
);
40+
});
41+
42+
it("preserves final answer content from mixed structured reasoning", () => {
43+
expect(
44+
sanitizeAssistantForDelivery({
45+
text: "Reasoning:\n_private step_\n\nFinal answer:\nVisible answer",
46+
}),
47+
).toEqual({ text: "Visible answer" });
48+
});
49+
50+
it("does not alter tagged thinking inside fenced code blocks", () => {
51+
expect(
52+
sanitizeAssistantTextForDelivery(
53+
"```xml\n<think>example</think>\n```\n<think>hidden</think>Visible",
54+
),
55+
).toBe("```xml\n<think>example</think>\n```\nVisible");
56+
});
57+
});
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
import { copyReplyPayloadMetadata } from "../../auto-reply/reply-payload.js";
2+
import type { ReplyPayload } from "../../auto-reply/types.js";
3+
import { hasReplyPayloadContent } from "../../interactive/payload.js";
4+
5+
export type AssistantDeliverySanitizerRole = "assistant" | "user" | "system" | "tool";
6+
7+
export type AssistantDeliverySanitizerOptions = {
8+
role?: AssistantDeliverySanitizerRole;
9+
/**
10+
* When true, visible channel surfaces may keep tagged thinking text. The
11+
* default is false because most outbound channels do not have a separate
12+
* reasoning lane and must never expose model-private reasoning accidentally.
13+
*/
14+
thinkingEnabled?: boolean;
15+
reasoningEnabled?: boolean;
16+
};
17+
18+
const THINKING_TAG_NAMES = ["think", "thinking", "reasoning", "thought", "thoughts"];
19+
const THINKING_TAG_PATTERN = new RegExp(
20+
`<\\s*(${THINKING_TAG_NAMES.join("|")})\\b[^>]*>[\\s\\S]*?<\\s*/\\s*\\1\\s*>`,
21+
"gi",
22+
);
23+
const FENCE_PATTERN = /^\s*(```|~~~)/;
24+
const MIXED_REASONING_FINAL_PATTERN =
25+
/^\s*(?:reasoning|thinking|thoughts?)\s*:\s*[\s\S]*?(?:\n\s*(?:final(?:\s+answer)?|answer|response)\s*:\s*)([\s\S]+)$/i;
26+
const REASONING_ONLY_PATTERN = /^\s*(?:reasoning|thinking|thoughts?)\s*:\s*[\s\S]*$/i;
27+
28+
function canShowThinking(options: AssistantDeliverySanitizerOptions): boolean {
29+
return options.thinkingEnabled === true || options.reasoningEnabled === true;
30+
}
31+
32+
function isAssistantRole(options: AssistantDeliverySanitizerOptions): boolean {
33+
return (options.role ?? "assistant") === "assistant";
34+
}
35+
36+
function stripTaggedThinkingOutsideCodeBlocks(text: string): string {
37+
const lines = text.split(/(?<=\n)/);
38+
let inFence = false;
39+
let changed = false;
40+
let outsideBuffer = "";
41+
const out: string[] = [];
42+
const flushOutside = () => {
43+
if (!outsideBuffer) {
44+
return;
45+
}
46+
const stripped = outsideBuffer.replace(THINKING_TAG_PATTERN, "");
47+
changed ||= stripped !== outsideBuffer;
48+
out.push(stripped);
49+
outsideBuffer = "";
50+
};
51+
for (const line of lines) {
52+
if (FENCE_PATTERN.test(line)) {
53+
flushOutside();
54+
inFence = !inFence;
55+
out.push(line);
56+
continue;
57+
}
58+
if (inFence) {
59+
out.push(line);
60+
continue;
61+
}
62+
outsideBuffer += line;
63+
}
64+
flushOutside();
65+
return changed ? out.join("") : text;
66+
}
67+
68+
export function sanitizeAssistantTextForDelivery(
69+
text: string,
70+
options: AssistantDeliverySanitizerOptions = {},
71+
): string | null {
72+
if (!isAssistantRole(options) || canShowThinking(options)) {
73+
return text;
74+
}
75+
if (!text.trim()) {
76+
return text;
77+
}
78+
let sanitized = stripTaggedThinkingOutsideCodeBlocks(text);
79+
const mixedReasoning = MIXED_REASONING_FINAL_PATTERN.exec(sanitized);
80+
if (mixedReasoning?.[1]?.trim()) {
81+
sanitized = mixedReasoning[1];
82+
} else if (REASONING_ONLY_PATTERN.test(sanitized)) {
83+
return null;
84+
}
85+
sanitized = sanitized.replace(/\n{3,}/g, "\n\n").trim();
86+
return sanitized ? sanitized : null;
87+
}
88+
89+
export function sanitizeAssistantForDelivery<T extends ReplyPayload>(
90+
payload: T,
91+
options: AssistantDeliverySanitizerOptions = {},
92+
): T | null {
93+
if (!isAssistantRole(options)) {
94+
return payload;
95+
}
96+
if (payload.isReasoning === true && !canShowThinking(options)) {
97+
return null;
98+
}
99+
if (typeof payload.text !== "string") {
100+
return payload;
101+
}
102+
const text = sanitizeAssistantTextForDelivery(payload.text, options);
103+
if (text === payload.text) {
104+
return payload;
105+
}
106+
if (text === null) {
107+
const withoutText = copyReplyPayloadMetadata(payload, { ...payload, text: undefined });
108+
return hasReplyPayloadContent(withoutText) ? (withoutText as T) : null;
109+
}
110+
return copyReplyPayloadMetadata(payload, { ...payload, text }) as T;
111+
}

src/infra/outbound/deliver.test.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,42 @@ describe("deliverOutboundPayloads", () => {
360360
expect(results).toEqual([{ channel: "matrix", messageId: "m1", roomId: "!room:example" }]);
361361
});
362362

363+
it("sanitizes assistant thinking tags before channel send", async () => {
364+
const sendMatrix = vi.fn().mockResolvedValue({ messageId: "m1", roomId: "!room:example" });
365+
366+
await deliverMatrixPayload({
367+
sendMatrix,
368+
payload: { text: "<think>hidden</think>Visible" },
369+
});
370+
371+
expect(sendMatrix).toHaveBeenCalledTimes(1);
372+
expect(requireMatrixSendCall(sendMatrix)[1]).toBe("Visible");
373+
});
374+
375+
it("drops reasoning-only assistant payloads before channel send", async () => {
376+
const sendMatrix = vi.fn().mockResolvedValue({ messageId: "m1", roomId: "!room:example" });
377+
378+
const results = await deliverMatrixPayload({
379+
sendMatrix,
380+
payload: { text: "Reasoning:\n_private step_" },
381+
});
382+
383+
expect(results).toEqual([]);
384+
expect(sendMatrix).not.toHaveBeenCalled();
385+
});
386+
387+
it("delivers only the answer for mixed reasoning and final assistant payloads", async () => {
388+
const sendMatrix = vi.fn().mockResolvedValue({ messageId: "m1", roomId: "!room:example" });
389+
390+
await deliverMatrixPayload({
391+
sendMatrix,
392+
payload: { text: "Reasoning:\n_private step_\n\nFinal answer:\nVisible" },
393+
});
394+
395+
expect(sendMatrix).toHaveBeenCalledTimes(1);
396+
expect(requireMatrixSendCall(sendMatrix)[1]).toBe("Visible");
397+
});
398+
363399
it("reports unsupported durable final delivery when required capabilities are missing", async () => {
364400
setActivePluginRegistry(
365401
createTestRegistry([

src/infra/outbound/deliver.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ import { diagnosticErrorCategory } from "../diagnostic-error-metadata.js";
4242
import { emitDiagnosticEvent, type DiagnosticMessageDeliveryKind } from "../diagnostic-events.js";
4343
import { formatErrorMessage } from "../errors.js";
4444
import { throwIfAborted } from "./abort.js";
45+
import { sanitizeAssistantForDelivery } from "./assistant-delivery-sanitizer.js";
4546
import { resolveOutboundChannelMessageAdapter } from "./channel-resolution.js";
4647
import {
4748
OutboundDeliveryError,
@@ -786,9 +787,12 @@ function normalizePayloadsForChannelDelivery(
786787
const normalizedPayload = handler.normalizePayload
787788
? handler.normalizePayload(sanitizedPayload)
788789
: sanitizedPayload;
789-
const normalized = normalizedPayload
790+
const deliverySanitizedPayload = normalizedPayload
791+
? sanitizeAssistantForDelivery(normalizedPayload, { role: "assistant" })
792+
: null;
793+
const normalized = deliverySanitizedPayload
790794
? normalizeEmptyPayloadForDelivery(
791-
stripInternalRuntimeScaffoldingFromPayload(normalizedPayload),
795+
stripInternalRuntimeScaffoldingFromPayload(deliverySanitizedPayload),
792796
)
793797
: null;
794798
if (normalized) {
@@ -1542,9 +1546,12 @@ async function deliverOutboundPayloadsCore(
15421546
const normalizedEffectivePayload = handler.normalizePayload
15431547
? handler.normalizePayload(renderedPayload)
15441548
: renderedPayload;
1545-
const effectivePayload = normalizedEffectivePayload
1549+
const deliverySanitizedEffectivePayload = normalizedEffectivePayload
1550+
? sanitizeAssistantForDelivery(normalizedEffectivePayload, { role: "assistant" })
1551+
: null;
1552+
const effectivePayload = deliverySanitizedEffectivePayload
15461553
? normalizeEmptyPayloadForDelivery(
1547-
stripInternalRuntimeScaffoldingFromPayload(normalizedEffectivePayload),
1554+
stripInternalRuntimeScaffoldingFromPayload(deliverySanitizedEffectivePayload),
15481555
)
15491556
: null;
15501557
if (!effectivePayload) {

0 commit comments

Comments
 (0)