mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 07:00:43 +00:00
fix(discord): run message_sending hooks for replies
Fixes Discord reply delivery so `message_sending` plugin hooks can transform or cancel outbound Discord replies, including DM targets. Fixes #59350. Thanks @wei840222.
This commit is contained in:
@@ -10,6 +10,7 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
### Fixes
|
||||
|
||||
- Discord/replies: run `message_sending` plugin hooks for Discord reply delivery, including DM targets, so plugins can transform or cancel outbound Discord replies consistently with other channels. Fixes #59350. (#71094) Thanks @wei840222.
|
||||
- Providers/GitHub Copilot: keep the plugin stream wrapper from claiming transport selection before OpenClaw picks a boundary-aware stream path, avoiding Pi's stale fallback Copilot headers on normal model turns. Thanks @steipete.
|
||||
- Discord/subagents: pass runtime config into thread-bound native subagent binding and require it at the helper boundary so Discord channel resolution keeps account-aware config. Fixes #71054. (#70945) Thanks @jai.
|
||||
- Slack/Assistant: accept Slack Assistant DM `message_changed` events when their metadata identifies the human sender, while continuing to drop self-authored bot edits. Fixes #55445. Thanks @AlfredPros.
|
||||
|
||||
@@ -11,6 +11,10 @@ const sendMessageDiscordMock = vi.hoisted(() => vi.fn());
|
||||
const sendVoiceMessageDiscordMock = vi.hoisted(() => vi.fn());
|
||||
const sendWebhookMessageDiscordMock = vi.hoisted(() => vi.fn());
|
||||
const sendDiscordTextMock = vi.hoisted(() => vi.fn());
|
||||
const messageHookRunner = vi.hoisted(() => ({
|
||||
hasHooks: vi.fn<(name: string) => boolean>(() => false),
|
||||
runMessageSending: vi.fn(),
|
||||
}));
|
||||
const buildDiscordSendErrorMock = vi.hoisted(() =>
|
||||
vi.fn<(err: unknown, ctx?: unknown) => Promise<unknown>>(async (err: unknown) => err),
|
||||
);
|
||||
@@ -65,6 +69,10 @@ vi.mock("openclaw/plugin-sdk/retry-runtime", async () => {
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("openclaw/plugin-sdk/plugin-runtime", () => ({
|
||||
getGlobalHookRunner: () => messageHookRunner,
|
||||
}));
|
||||
|
||||
let deliverDiscordReply: typeof import("./reply-delivery.js").deliverDiscordReply;
|
||||
|
||||
describe("deliverDiscordReply", () => {
|
||||
@@ -143,6 +151,8 @@ describe("deliverDiscordReply", () => {
|
||||
});
|
||||
buildDiscordSendErrorMock.mockClear().mockImplementation(async (err: unknown) => err);
|
||||
retryAsyncMock.mockClear();
|
||||
messageHookRunner.hasHooks.mockReset().mockReturnValue(false);
|
||||
messageHookRunner.runMessageSending.mockReset();
|
||||
threadBindingTesting.resetThreadBindingsForTests();
|
||||
});
|
||||
|
||||
@@ -668,4 +678,201 @@ describe("deliverDiscordReply", () => {
|
||||
expect.objectContaining({ token: "token", accountId: "default" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("replaces reply text with message_sending hook content", async () => {
|
||||
messageHookRunner.hasHooks.mockImplementation((name: string) => name === "message_sending");
|
||||
messageHookRunner.runMessageSending.mockResolvedValue({ content: "filtered text" });
|
||||
|
||||
await deliverDiscordReply({
|
||||
replies: [{ text: "raw secret" }],
|
||||
target: "channel:hook-1",
|
||||
token: "token",
|
||||
accountId: "acc-1",
|
||||
runtime,
|
||||
cfg,
|
||||
textLimit: 2000,
|
||||
});
|
||||
|
||||
expect(messageHookRunner.runMessageSending).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
to: "hook-1",
|
||||
content: "raw secret",
|
||||
metadata: expect.objectContaining({ channel: "discord" }),
|
||||
}),
|
||||
expect.objectContaining({
|
||||
channelId: "discord",
|
||||
accountId: "acc-1",
|
||||
conversationId: "hook-1",
|
||||
}),
|
||||
);
|
||||
expect(sendMessageDiscordMock).toHaveBeenCalledTimes(1);
|
||||
expect(sendMessageDiscordMock).toHaveBeenCalledWith(
|
||||
"channel:hook-1",
|
||||
"filtered text",
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
it("uses the raw Discord target as hook destination for DMs", async () => {
|
||||
messageHookRunner.hasHooks.mockImplementation((name: string) => name === "message_sending");
|
||||
messageHookRunner.runMessageSending.mockResolvedValue({ content: "dm filtered" });
|
||||
|
||||
await deliverDiscordReply({
|
||||
replies: [{ text: "dm raw" }],
|
||||
target: "user:U123",
|
||||
token: "token",
|
||||
accountId: "acc-1",
|
||||
runtime,
|
||||
cfg,
|
||||
textLimit: 2000,
|
||||
});
|
||||
|
||||
expect(messageHookRunner.runMessageSending).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
to: "user:U123",
|
||||
content: "dm raw",
|
||||
}),
|
||||
expect.objectContaining({
|
||||
channelId: "discord",
|
||||
accountId: "acc-1",
|
||||
conversationId: "user:U123",
|
||||
}),
|
||||
);
|
||||
expect(sendMessageDiscordMock).toHaveBeenCalledWith(
|
||||
"user:U123",
|
||||
"dm filtered",
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
it("reports replyToId to hooks only while a single-use fallback reply is still available", async () => {
|
||||
messageHookRunner.hasHooks.mockImplementation((name: string) => name === "message_sending");
|
||||
messageHookRunner.runMessageSending.mockResolvedValue({});
|
||||
|
||||
await deliverDiscordReply({
|
||||
replies: [{ text: "first" }, { text: "second" }],
|
||||
target: "channel:hook-thread",
|
||||
token: "token",
|
||||
runtime,
|
||||
cfg,
|
||||
textLimit: 2000,
|
||||
replyToId: "reply-1",
|
||||
replyToMode: "first",
|
||||
});
|
||||
|
||||
expect(messageHookRunner.runMessageSending).toHaveBeenCalledTimes(2);
|
||||
expect(messageHookRunner.runMessageSending.mock.calls[0]?.[0]).toEqual(
|
||||
expect.objectContaining({ replyToId: "reply-1" }),
|
||||
);
|
||||
expect(messageHookRunner.runMessageSending.mock.calls[1]?.[0]).toEqual(
|
||||
expect.not.objectContaining({ replyToId: expect.anything() }),
|
||||
);
|
||||
expect(sendMessageDiscordMock.mock.calls[0]?.[2]).toEqual(
|
||||
expect.objectContaining({ replyTo: "reply-1" }),
|
||||
);
|
||||
expect(sendMessageDiscordMock.mock.calls[1]?.[2]).toEqual(
|
||||
expect.not.objectContaining({ replyTo: expect.anything() }),
|
||||
);
|
||||
});
|
||||
|
||||
it("reports explicit payload reply targets to hooks when replyToMode is off", async () => {
|
||||
messageHookRunner.hasHooks.mockImplementation((name: string) => name === "message_sending");
|
||||
messageHookRunner.runMessageSending.mockResolvedValue({});
|
||||
|
||||
await deliverDiscordReply({
|
||||
replies: [
|
||||
{
|
||||
text: "explicit",
|
||||
replyToId: "reply-explicit-1",
|
||||
replyToTag: true,
|
||||
},
|
||||
],
|
||||
target: "channel:hook-explicit",
|
||||
token: "token",
|
||||
runtime,
|
||||
cfg,
|
||||
textLimit: 2000,
|
||||
replyToMode: "off",
|
||||
});
|
||||
|
||||
expect(messageHookRunner.runMessageSending.mock.calls[0]?.[0]).toEqual(
|
||||
expect.objectContaining({ replyToId: "reply-explicit-1" }),
|
||||
);
|
||||
expect(sendMessageDiscordMock.mock.calls[0]?.[2]).toEqual(
|
||||
expect.objectContaining({ replyTo: "reply-explicit-1" }),
|
||||
);
|
||||
});
|
||||
|
||||
it("skips delivery when message_sending hook cancels the payload", async () => {
|
||||
messageHookRunner.hasHooks.mockImplementation((name: string) => name === "message_sending");
|
||||
messageHookRunner.runMessageSending.mockResolvedValue({ cancel: true });
|
||||
|
||||
await deliverDiscordReply({
|
||||
replies: [{ text: "should not send" }],
|
||||
target: "channel:hook-2",
|
||||
token: "token",
|
||||
runtime,
|
||||
cfg,
|
||||
textLimit: 2000,
|
||||
});
|
||||
|
||||
expect(sendMessageDiscordMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("skips delivery when hook blanks out a text-only reply", async () => {
|
||||
messageHookRunner.hasHooks.mockImplementation((name: string) => name === "message_sending");
|
||||
messageHookRunner.runMessageSending.mockResolvedValue({ content: " " });
|
||||
|
||||
await deliverDiscordReply({
|
||||
replies: [{ text: "hello" }],
|
||||
target: "channel:hook-3",
|
||||
token: "token",
|
||||
runtime,
|
||||
cfg,
|
||||
textLimit: 2000,
|
||||
});
|
||||
|
||||
expect(sendMessageDiscordMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("continues delivery when message_sending hook throws", async () => {
|
||||
messageHookRunner.hasHooks.mockImplementation((name: string) => name === "message_sending");
|
||||
messageHookRunner.runMessageSending.mockRejectedValue(new Error("plugin exploded"));
|
||||
const errorRuntime = { error: vi.fn() } as unknown as RuntimeEnv;
|
||||
|
||||
await deliverDiscordReply({
|
||||
replies: [{ text: "should still send" }],
|
||||
target: "channel:hook-4",
|
||||
token: "token",
|
||||
runtime: errorRuntime,
|
||||
cfg,
|
||||
textLimit: 2000,
|
||||
});
|
||||
|
||||
expect(sendMessageDiscordMock).toHaveBeenCalledTimes(1);
|
||||
expect(sendMessageDiscordMock).toHaveBeenCalledWith(
|
||||
"channel:hook-4",
|
||||
"should still send",
|
||||
expect.anything(),
|
||||
);
|
||||
expect((errorRuntime.error as ReturnType<typeof vi.fn>).mock.calls[0]?.[0]).toMatch(
|
||||
/plugin exploded/,
|
||||
);
|
||||
});
|
||||
|
||||
it("skips hook resolution when no message_sending hooks are registered", async () => {
|
||||
messageHookRunner.hasHooks.mockReturnValue(false);
|
||||
|
||||
await deliverDiscordReply({
|
||||
replies: [{ text: "no hook path" }],
|
||||
target: "channel:hook-5",
|
||||
token: "token",
|
||||
runtime,
|
||||
cfg,
|
||||
textLimit: 2000,
|
||||
});
|
||||
|
||||
expect(messageHookRunner.runMessageSending).not.toHaveBeenCalled();
|
||||
expect(sendMessageDiscordMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2,6 +2,7 @@ import type { RequestClient } from "@buape/carbon";
|
||||
import { resolveAgentAvatar } from "openclaw/plugin-sdk/agent-runtime";
|
||||
import type { OpenClawConfig } from "openclaw/plugin-sdk/config-runtime";
|
||||
import type { MarkdownTableMode, ReplyToMode } from "openclaw/plugin-sdk/config-runtime";
|
||||
import { getGlobalHookRunner } from "openclaw/plugin-sdk/plugin-runtime";
|
||||
import type { ChunkMode } from "openclaw/plugin-sdk/reply-chunking";
|
||||
import type { ReplyPayload } from "openclaw/plugin-sdk/reply-dispatch-runtime";
|
||||
import {
|
||||
@@ -250,6 +251,28 @@ function createPayloadReplyToResolver(params: {
|
||||
};
|
||||
}
|
||||
|
||||
function resolveMessageSendingHookReplyToId(params: {
|
||||
payload: ReplyPayload;
|
||||
replyToMode: ReplyToMode;
|
||||
fallbackReplyTo: string | undefined;
|
||||
fallbackReplyUsed: boolean;
|
||||
}): string | undefined {
|
||||
const payloadReplyTo = normalizeOptionalString(params.payload.replyToId);
|
||||
const allowExplicitReplyWhenOff = Boolean(
|
||||
payloadReplyTo && (params.payload.replyToTag || params.payload.replyToCurrent),
|
||||
);
|
||||
if (payloadReplyTo && (params.replyToMode !== "off" || allowExplicitReplyWhenOff)) {
|
||||
return payloadReplyTo;
|
||||
}
|
||||
if (!params.fallbackReplyTo) {
|
||||
return undefined;
|
||||
}
|
||||
if (!isSingleUseReplyToMode(params.replyToMode)) {
|
||||
return params.fallbackReplyTo;
|
||||
}
|
||||
return params.fallbackReplyUsed ? undefined : params.fallbackReplyTo;
|
||||
}
|
||||
|
||||
function resolveBindingPersona(
|
||||
cfg: OpenClawConfig,
|
||||
binding: DiscordThreadBindingLookupRecord | undefined,
|
||||
@@ -412,6 +435,9 @@ export async function deliverDiscordReply(params: {
|
||||
const request: RetryRunner | undefined = channelId
|
||||
? createDiscordRetryRunner({ configRetry: account.config.retry })
|
||||
: undefined;
|
||||
const hookRunner = getGlobalHookRunner();
|
||||
const hasMessageSendingHooks = hookRunner?.hasHooks("message_sending") ?? false;
|
||||
const hookConversationId = channelId ?? params.target;
|
||||
let deliveredAny = false;
|
||||
for (const payload of params.replies) {
|
||||
const resolvePayloadReplyTo = createPayloadReplyToResolver({
|
||||
@@ -420,9 +446,46 @@ export async function deliverDiscordReply(params: {
|
||||
resolveFallbackReplyTo: resolveReplyTo,
|
||||
});
|
||||
const tableMode = params.tableMode ?? "code";
|
||||
const reply = resolveSendableOutboundReplyParts(payload, {
|
||||
text: convertMarkdownTables(payload.text ?? "", tableMode),
|
||||
});
|
||||
let effectiveText = payload.text ?? "";
|
||||
if (hasMessageSendingHooks) {
|
||||
try {
|
||||
const hookResult = await hookRunner?.runMessageSending(
|
||||
{
|
||||
to: hookConversationId,
|
||||
content: effectiveText,
|
||||
replyToId: resolveMessageSendingHookReplyToId({
|
||||
payload,
|
||||
replyToMode,
|
||||
fallbackReplyTo: replyTo,
|
||||
fallbackReplyUsed: replyUsed,
|
||||
}),
|
||||
metadata: {
|
||||
channel: "discord",
|
||||
mediaUrls: payload.mediaUrls ?? (payload.mediaUrl ? [payload.mediaUrl] : undefined),
|
||||
},
|
||||
},
|
||||
{
|
||||
channelId: "discord",
|
||||
accountId: params.accountId,
|
||||
conversationId: hookConversationId,
|
||||
},
|
||||
);
|
||||
if (hookResult?.cancel) {
|
||||
continue;
|
||||
}
|
||||
if (typeof hookResult?.content === "string") {
|
||||
effectiveText = hookResult.content;
|
||||
}
|
||||
} catch (error) {
|
||||
params.runtime.error?.(
|
||||
`discord: message_sending hook failed: ${error instanceof Error ? error.message : String(error)}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
const reply = resolveSendableOutboundReplyParts(
|
||||
{ ...payload, text: effectiveText },
|
||||
{ text: convertMarkdownTables(effectiveText, tableMode) },
|
||||
);
|
||||
if (!reply.hasContent) {
|
||||
continue;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user