mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-19 14:31:39 +00:00
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 <li.yuanbin1@xydigit.com> * 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 <266665499+Pick-cat@users.noreply.github.com> Co-authored-by: Peter Steinberger <steipete@gmail.com> Co-authored-by: liyuanbin <li.yuanbin1@xydigit.com>
This commit is contained in:
@@ -8,6 +8,7 @@ import {
|
||||
createComputedAccountStatusAdapter,
|
||||
createDefaultChannelRuntimeState,
|
||||
} from "openclaw/plugin-sdk/status-helpers";
|
||||
import { sanitizeAssistantVisibleText } from "openclaw/plugin-sdk/text-chunking";
|
||||
import { resolveNextcloudTalkAccount, type ResolvedNextcloudTalkAccount } from "./accounts.js";
|
||||
import { nextcloudTalkApprovalAuth } from "./approval-auth.js";
|
||||
import { probeNextcloudTalkBotResponseFeature } from "./bot-preflight.js";
|
||||
@@ -201,6 +202,7 @@ export const nextcloudTalkPlugin: ChannelPlugin<ResolvedNextcloudTalkAccount> =
|
||||
getNextcloudTalkRuntime().channel.text.chunkMarkdownText(text, limit),
|
||||
chunkerMode: "markdown",
|
||||
textChunkLimit: 4000,
|
||||
sanitizeText: ({ text }) => sanitizeAssistantVisibleText(text),
|
||||
},
|
||||
attachedResults: {
|
||||
channel: "nextcloud-talk",
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
// Nextcloud Talk tests cover inbound.behavior plugin behavior.
|
||||
import { createPluginRuntimeMock } from "openclaw/plugin-sdk/channel-test-helpers";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import type { PluginRuntime, RuntimeEnv } from "../runtime-api.js";
|
||||
import type { OutboundReplyPayload, PluginRuntime, RuntimeEnv } from "../runtime-api.js";
|
||||
import type { ResolvedNextcloudTalkAccount } from "./accounts.js";
|
||||
import { handleNextcloudTalkInbound } from "./inbound.js";
|
||||
import { setNextcloudTalkRuntime } from "./runtime.js";
|
||||
@@ -307,4 +307,73 @@ describe("nextcloud-talk inbound behavior", () => {
|
||||
) as { replyPipeline?: unknown };
|
||||
expect(assembledRequest.replyPipeline).toEqual({});
|
||||
});
|
||||
|
||||
it("sanitizes inbound replies before local delivery while preserving transport fields", async () => {
|
||||
const coreRuntime = createPluginRuntimeMock();
|
||||
setNextcloudTalkRuntime(coreRuntime as unknown as PluginRuntime);
|
||||
createChannelPairingControllerMock.mockReturnValue({
|
||||
readStoreForDmPolicy: vi.fn(async () => []),
|
||||
issueChallenge: vi.fn(),
|
||||
});
|
||||
sendMessageNextcloudTalkMock.mockResolvedValue(undefined);
|
||||
|
||||
const config = { channels: { "nextcloud-talk": {} } } as CoreConfig;
|
||||
await handleNextcloudTalkInbound({
|
||||
message: createMessage(),
|
||||
account: createAccount({
|
||||
config: {
|
||||
dmPolicy: "allowlist",
|
||||
allowFrom: ["user-1"],
|
||||
groupPolicy: "allowlist",
|
||||
groupAllowFrom: [],
|
||||
},
|
||||
}),
|
||||
config,
|
||||
runtime: createRuntimeEnv(),
|
||||
});
|
||||
|
||||
const assembledRequest = requireFirstMockArg(
|
||||
coreRuntime.channel.inbound.dispatchReply as ReturnType<typeof vi.fn>,
|
||||
"Nextcloud Talk assembled request",
|
||||
) as {
|
||||
delivery?: {
|
||||
preparePayload?: (payload: OutboundReplyPayload) => OutboundReplyPayload;
|
||||
deliver?: (payload: OutboundReplyPayload) => Promise<{ visibleReplySent: boolean }>;
|
||||
};
|
||||
};
|
||||
const preparePayload = assembledRequest.delivery?.preparePayload;
|
||||
const deliver = assembledRequest.delivery?.deliver;
|
||||
if (!preparePayload || !deliver) {
|
||||
throw new Error("expected Nextcloud Talk reply delivery hooks");
|
||||
}
|
||||
|
||||
const mediaOnlyPayload = { mediaUrl: "https://example.com/a.png" };
|
||||
expect(preparePayload(mediaOnlyPayload)).toBe(mediaOnlyPayload);
|
||||
|
||||
const preparedPayload = preparePayload({
|
||||
text: "Done.\n⚠️ 🛠️ `search repos (agent)` failed",
|
||||
mediaUrls: ["https://example.com/a.png"],
|
||||
replyToId: "reply-1",
|
||||
});
|
||||
expect(preparedPayload).toEqual({
|
||||
text: "Done.",
|
||||
mediaUrls: ["https://example.com/a.png"],
|
||||
replyToId: "reply-1",
|
||||
});
|
||||
await expect(deliver(preparedPayload)).resolves.toEqual({ visibleReplySent: true });
|
||||
await expect(
|
||||
deliver(preparePayload({ text: "⚠️ 🛠️ `search repos (agent)` failed" })),
|
||||
).resolves.toEqual({ visibleReplySent: false });
|
||||
|
||||
expect(sendMessageNextcloudTalkMock).toHaveBeenCalledTimes(1);
|
||||
expect(requireFirstSendMessageCall()).toEqual([
|
||||
"room-1",
|
||||
"Done.\n\nAttachment: https://example.com/a.png",
|
||||
{
|
||||
cfg: config,
|
||||
accountId: "default",
|
||||
replyTo: "reply-1",
|
||||
},
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -8,6 +8,7 @@ import {
|
||||
normalizeOptionalString,
|
||||
normalizeStringEntries,
|
||||
} from "openclaw/plugin-sdk/string-coerce-runtime";
|
||||
import { sanitizeAssistantVisibleText } from "openclaw/plugin-sdk/text-chunking";
|
||||
import {
|
||||
GROUP_POLICY_BLOCKED_LABEL,
|
||||
resolveAllowlistProviderRuntimeGroupPolicy,
|
||||
@@ -97,9 +98,9 @@ async function deliverNextcloudTalkReply(params: {
|
||||
roomToken: string;
|
||||
accountId: string;
|
||||
statusSink?: (patch: { lastOutboundAt?: number }) => void;
|
||||
}): Promise<void> {
|
||||
}): Promise<{ visibleReplySent: boolean }> {
|
||||
const { cfg, payload, roomToken, accountId, statusSink } = params;
|
||||
await deliverFormattedTextWithAttachments({
|
||||
const visibleReplySent = await deliverFormattedTextWithAttachments({
|
||||
payload,
|
||||
send: async ({ text, replyToId }) => {
|
||||
await sendMessageNextcloudTalk(roomToken, text, {
|
||||
@@ -110,6 +111,7 @@ async function deliverNextcloudTalkReply(params: {
|
||||
statusSink?.({ lastOutboundAt: Date.now() });
|
||||
},
|
||||
});
|
||||
return { visibleReplySent };
|
||||
}
|
||||
|
||||
export async function handleNextcloudTalkInbound(params: {
|
||||
@@ -361,8 +363,15 @@ export async function handleNextcloudTalkInbound(params: {
|
||||
dispatchReplyWithBufferedBlockDispatcher:
|
||||
core.channel.reply.dispatchReplyWithBufferedBlockDispatcher,
|
||||
delivery: {
|
||||
preparePayload: (payload) =>
|
||||
payload.text === undefined
|
||||
? payload
|
||||
: {
|
||||
...payload,
|
||||
text: sanitizeAssistantVisibleText(payload.text),
|
||||
},
|
||||
deliver: async (payload) => {
|
||||
await deliverNextcloudTalkReply({
|
||||
return await deliverNextcloudTalkReply({
|
||||
cfg: config,
|
||||
payload,
|
||||
roomToken,
|
||||
|
||||
@@ -0,0 +1,45 @@
|
||||
// Nextcloud Talk outbound must strip assistant internal tool-trace scaffolding
|
||||
// before delivery, matching the shared channel sanitizer contract.
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { nextcloudTalkPlugin } from "./channel.js";
|
||||
|
||||
function sanitizeOutboundText(text: string): string {
|
||||
const sanitizeText = nextcloudTalkPlugin.outbound?.sanitizeText;
|
||||
if (!sanitizeText) {
|
||||
throw new Error("Expected Nextcloud Talk outbound sanitizeText hook");
|
||||
}
|
||||
return sanitizeText({ text, payload: { text } });
|
||||
}
|
||||
|
||||
describe("nextcloud-talk outbound sanitizeText", () => {
|
||||
it("strips internal tool-trace banners before outbound delivery", () => {
|
||||
const text = "Done.\n⚠️ 🛠️ `search repos (agent)` failed";
|
||||
expect(sanitizeOutboundText(text)).toBe("Done.");
|
||||
});
|
||||
|
||||
it("strips XML tool-call scaffolding leaked into assistant text", () => {
|
||||
const text = '<tool_call>{"name":"exec"}</tool_call>Meeting notes sent.';
|
||||
expect(sanitizeOutboundText(text)).toBe("Meeting notes sent.");
|
||||
});
|
||||
|
||||
it("strips multiline tool-response scaffolding leaked into assistant text", () => {
|
||||
const text = [
|
||||
"Checking now.",
|
||||
"<function_response>",
|
||||
'Searching for: "agenda"',
|
||||
"</function_response>",
|
||||
"Meeting notes sent.",
|
||||
].join("\n");
|
||||
expect(sanitizeOutboundText(text)).toBe("Checking now.\n\nMeeting notes sent.");
|
||||
});
|
||||
|
||||
it("preserves ordinary assistant prose while sanitizing", () => {
|
||||
const text = "The agenda has 3 open action items.";
|
||||
expect(sanitizeOutboundText(text)).toBe(text);
|
||||
});
|
||||
|
||||
it("preserves internal trace examples inside fenced code", () => {
|
||||
const text = ["Example:", "```", "⚠️ 🛠️ `search repos (agent)` failed", "```"].join("\n");
|
||||
expect(sanitizeOutboundText(text)).toBe(text);
|
||||
});
|
||||
});
|
||||
@@ -149,6 +149,26 @@ describe("nextcloud-talk send cfg threading", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves caller-authored text on the low-level send path", async () => {
|
||||
const cfg = { source: "provided" } as const;
|
||||
const text = "Example:\n⚠️ 🛠️ `search repos (agent)` failed";
|
||||
mockNextcloudMessageResponse(12346, 1_706_000_001);
|
||||
|
||||
await sendMessageNextcloudTalk("room:abc123", text, {
|
||||
cfg,
|
||||
accountId: "work",
|
||||
replyTo: "parent-1",
|
||||
});
|
||||
|
||||
expect(hoisted.generateNextcloudTalkSignature).toHaveBeenCalledWith({
|
||||
body: text,
|
||||
secret: "secret-value",
|
||||
});
|
||||
expect(fetchMock.mock.calls[0]?.[1]?.body).toBe(
|
||||
JSON.stringify({ message: text, replyTo: "parent-1" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("sends with provided cfg even when the runtime store is not initialized", async () => {
|
||||
const cfg = { source: "provided" } as const;
|
||||
hoisted.record.mockImplementation(() => {
|
||||
|
||||
Reference in New Issue
Block a user