Skip to content

Commit 8a93d28

Browse files
Pick-catsteipetemiorbnli
authored
fix(nextcloud-talk): strip internal tool-trace banners from outbound text (#101712)
* fix(nextcloud-talk): strip internal tool-trace banners from outbound text * fix(nextcloud-talk): sanitize inbound replies Co-authored-by: liyuanbin <[email protected]> * test(nextcloud-talk): prove low-level send text preservation * test(nextcloud-talk): focus inbound sanitizer coverage * fix(nextcloud-talk): report stripped replies as non-visible * docs(changelog): note Nextcloud Talk reply sanitization * chore: keep PR changelog-neutral --------- Co-authored-by: Pick-cat <[email protected]> Co-authored-by: Peter Steinberger <[email protected]> Co-authored-by: liyuanbin <[email protected]>
1 parent 3a64889 commit 8a93d28

5 files changed

Lines changed: 149 additions & 4 deletions

File tree

‎extensions/nextcloud-talk/src/channel.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
createComputedAccountStatusAdapter,
99
createDefaultChannelRuntimeState,
1010
} from "openclaw/plugin-sdk/status-helpers";
11+
import { sanitizeAssistantVisibleText } from "openclaw/plugin-sdk/text-chunking";
1112
import { resolveNextcloudTalkAccount, type ResolvedNextcloudTalkAccount } from "./accounts.js";
1213
import { nextcloudTalkApprovalAuth } from "./approval-auth.js";
1314
import { probeNextcloudTalkBotResponseFeature } from "./bot-preflight.js";
@@ -201,6 +202,7 @@ export const nextcloudTalkPlugin: ChannelPlugin<ResolvedNextcloudTalkAccount> =
201202
getNextcloudTalkRuntime().channel.text.chunkMarkdownText(text, limit),
202203
chunkerMode: "markdown",
203204
textChunkLimit: 4000,
205+
sanitizeText: ({ text }) => sanitizeAssistantVisibleText(text),
204206
},
205207
attachedResults: {
206208
channel: "nextcloud-talk",

‎extensions/nextcloud-talk/src/inbound.behavior.test.ts‎

Lines changed: 70 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Nextcloud Talk tests cover inbound.behavior plugin behavior.
22
import { createPluginRuntimeMock } from "openclaw/plugin-sdk/channel-test-helpers";
33
import { beforeEach, describe, expect, it, vi } from "vitest";
4-
import type { PluginRuntime, RuntimeEnv } from "../runtime-api.js";
4+
import type { OutboundReplyPayload, PluginRuntime, RuntimeEnv } from "../runtime-api.js";
55
import type { ResolvedNextcloudTalkAccount } from "./accounts.js";
66
import { handleNextcloudTalkInbound } from "./inbound.js";
77
import { setNextcloudTalkRuntime } from "./runtime.js";
@@ -307,4 +307,73 @@ describe("nextcloud-talk inbound behavior", () => {
307307
) as { replyPipeline?: unknown };
308308
expect(assembledRequest.replyPipeline).toEqual({});
309309
});
310+
311+
it("sanitizes inbound replies before local delivery while preserving transport fields", async () => {
312+
const coreRuntime = createPluginRuntimeMock();
313+
setNextcloudTalkRuntime(coreRuntime as unknown as PluginRuntime);
314+
createChannelPairingControllerMock.mockReturnValue({
315+
readStoreForDmPolicy: vi.fn(async () => []),
316+
issueChallenge: vi.fn(),
317+
});
318+
sendMessageNextcloudTalkMock.mockResolvedValue(undefined);
319+
320+
const config = { channels: { "nextcloud-talk": {} } } as CoreConfig;
321+
await handleNextcloudTalkInbound({
322+
message: createMessage(),
323+
account: createAccount({
324+
config: {
325+
dmPolicy: "allowlist",
326+
allowFrom: ["user-1"],
327+
groupPolicy: "allowlist",
328+
groupAllowFrom: [],
329+
},
330+
}),
331+
config,
332+
runtime: createRuntimeEnv(),
333+
});
334+
335+
const assembledRequest = requireFirstMockArg(
336+
coreRuntime.channel.inbound.dispatchReply as ReturnType<typeof vi.fn>,
337+
"Nextcloud Talk assembled request",
338+
) as {
339+
delivery?: {
340+
preparePayload?: (payload: OutboundReplyPayload) => OutboundReplyPayload;
341+
deliver?: (payload: OutboundReplyPayload) => Promise<{ visibleReplySent: boolean }>;
342+
};
343+
};
344+
const preparePayload = assembledRequest.delivery?.preparePayload;
345+
const deliver = assembledRequest.delivery?.deliver;
346+
if (!preparePayload || !deliver) {
347+
throw new Error("expected Nextcloud Talk reply delivery hooks");
348+
}
349+
350+
const mediaOnlyPayload = { mediaUrl: "https://example.com/a.png" };
351+
expect(preparePayload(mediaOnlyPayload)).toBe(mediaOnlyPayload);
352+
353+
const preparedPayload = preparePayload({
354+
text: "Done.\n⚠️ 🛠️ `search repos (agent)` failed",
355+
mediaUrls: ["https://example.com/a.png"],
356+
replyToId: "reply-1",
357+
});
358+
expect(preparedPayload).toEqual({
359+
text: "Done.",
360+
mediaUrls: ["https://example.com/a.png"],
361+
replyToId: "reply-1",
362+
});
363+
await expect(deliver(preparedPayload)).resolves.toEqual({ visibleReplySent: true });
364+
await expect(
365+
deliver(preparePayload({ text: "⚠️ 🛠️ `search repos (agent)` failed" })),
366+
).resolves.toEqual({ visibleReplySent: false });
367+
368+
expect(sendMessageNextcloudTalkMock).toHaveBeenCalledTimes(1);
369+
expect(requireFirstSendMessageCall()).toEqual([
370+
"room-1",
371+
"Done.\n\nAttachment: https://example.com/a.png",
372+
{
373+
cfg: config,
374+
accountId: "default",
375+
replyTo: "reply-1",
376+
},
377+
]);
378+
});
310379
});

‎extensions/nextcloud-talk/src/inbound.ts‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
normalizeOptionalString,
99
normalizeStringEntries,
1010
} from "openclaw/plugin-sdk/string-coerce-runtime";
11+
import { sanitizeAssistantVisibleText } from "openclaw/plugin-sdk/text-chunking";
1112
import {
1213
GROUP_POLICY_BLOCKED_LABEL,
1314
resolveAllowlistProviderRuntimeGroupPolicy,
@@ -97,9 +98,9 @@ async function deliverNextcloudTalkReply(params: {
9798
roomToken: string;
9899
accountId: string;
99100
statusSink?: (patch: { lastOutboundAt?: number }) => void;
100-
}): Promise<void> {
101+
}): Promise<{ visibleReplySent: boolean }> {
101102
const { cfg, payload, roomToken, accountId, statusSink } = params;
102-
await deliverFormattedTextWithAttachments({
103+
const visibleReplySent = await deliverFormattedTextWithAttachments({
103104
payload,
104105
send: async ({ text, replyToId }) => {
105106
await sendMessageNextcloudTalk(roomToken, text, {
@@ -110,6 +111,7 @@ async function deliverNextcloudTalkReply(params: {
110111
statusSink?.({ lastOutboundAt: Date.now() });
111112
},
112113
});
114+
return { visibleReplySent };
113115
}
114116

115117
export async function handleNextcloudTalkInbound(params: {
@@ -361,8 +363,15 @@ export async function handleNextcloudTalkInbound(params: {
361363
dispatchReplyWithBufferedBlockDispatcher:
362364
core.channel.reply.dispatchReplyWithBufferedBlockDispatcher,
363365
delivery: {
366+
preparePayload: (payload) =>
367+
payload.text === undefined
368+
? payload
369+
: {
370+
...payload,
371+
text: sanitizeAssistantVisibleText(payload.text),
372+
},
364373
deliver: async (payload) => {
365-
await deliverNextcloudTalkReply({
374+
return await deliverNextcloudTalkReply({
366375
cfg: config,
367376
payload,
368377
roomToken,

‎extensions/nextcloud-talk/src/outbound-tool-trace-sanitize.test.ts‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// Nextcloud Talk outbound must strip assistant internal tool-trace scaffolding
2+
// before delivery, matching the shared channel sanitizer contract.
3+
import { describe, expect, it } from "vitest";
4+
import { nextcloudTalkPlugin } from "./channel.js";
5+
6+
function sanitizeOutboundText(text: string): string {
7+
const sanitizeText = nextcloudTalkPlugin.outbound?.sanitizeText;
8+
if (!sanitizeText) {
9+
throw new Error("Expected Nextcloud Talk outbound sanitizeText hook");
10+
}
11+
return sanitizeText({ text, payload: { text } });
12+
}
13+
14+
describe("nextcloud-talk outbound sanitizeText", () => {
15+
it("strips internal tool-trace banners before outbound delivery", () => {
16+
const text = "Done.\n⚠️ 🛠️ `search repos (agent)` failed";
17+
expect(sanitizeOutboundText(text)).toBe("Done.");
18+
});
19+
20+
it("strips XML tool-call scaffolding leaked into assistant text", () => {
21+
const text = '<tool_call>{"name":"exec"}</tool_call>Meeting notes sent.';
22+
expect(sanitizeOutboundText(text)).toBe("Meeting notes sent.");
23+
});
24+
25+
it("strips multiline tool-response scaffolding leaked into assistant text", () => {
26+
const text = [
27+
"Checking now.",
28+
"<function_response>",
29+
'Searching for: "agenda"',
30+
"</function_response>",
31+
"Meeting notes sent.",
32+
].join("\n");
33+
expect(sanitizeOutboundText(text)).toBe("Checking now.\n\nMeeting notes sent.");
34+
});
35+
36+
it("preserves ordinary assistant prose while sanitizing", () => {
37+
const text = "The agenda has 3 open action items.";
38+
expect(sanitizeOutboundText(text)).toBe(text);
39+
});
40+
41+
it("preserves internal trace examples inside fenced code", () => {
42+
const text = ["Example:", "```", "⚠️ 🛠️ `search repos (agent)` failed", "```"].join("\n");
43+
expect(sanitizeOutboundText(text)).toBe(text);
44+
});
45+
});

‎extensions/nextcloud-talk/src/send.cfg-threading.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,26 @@ describe("nextcloud-talk send cfg threading", () => {
149149
});
150150
});
151151

152+
it("preserves caller-authored text on the low-level send path", async () => {
153+
const cfg = { source: "provided" } as const;
154+
const text = "Example:\n⚠️ 🛠️ `search repos (agent)` failed";
155+
mockNextcloudMessageResponse(12346, 1_706_000_001);
156+
157+
await sendMessageNextcloudTalk("room:abc123", text, {
158+
cfg,
159+
accountId: "work",
160+
replyTo: "parent-1",
161+
});
162+
163+
expect(hoisted.generateNextcloudTalkSignature).toHaveBeenCalledWith({
164+
body: text,
165+
secret: "secret-value",
166+
});
167+
expect(fetchMock.mock.calls[0]?.[1]?.body).toBe(
168+
JSON.stringify({ message: text, replyTo: "parent-1" }),
169+
);
170+
});
171+
152172
it("sends with provided cfg even when the runtime store is not initialized", async () => {
153173
const cfg = { source: "provided" } as const;
154174
hoisted.record.mockImplementation(() => {

0 commit comments

Comments
 (0)