From 3dba3d8b35ca141b362d2660c6c633f6e0c03363 Mon Sep 17 00:00:00 2001 From: wei Date: Sat, 25 Apr 2026 07:19:24 +0800 Subject: [PATCH] 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. --- CHANGELOG.md | 1 + .../src/monitor/reply-delivery.test.ts | 207 ++++++++++++++++++ .../discord/src/monitor/reply-delivery.ts | 69 +++++- 3 files changed, 274 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 94ea35123cb..d63aa28f42d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. diff --git a/extensions/discord/src/monitor/reply-delivery.test.ts b/extensions/discord/src/monitor/reply-delivery.test.ts index 389194fda72..074ce9b10ec 100644 --- a/extensions/discord/src/monitor/reply-delivery.test.ts +++ b/extensions/discord/src/monitor/reply-delivery.test.ts @@ -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>(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).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); + }); }); diff --git a/extensions/discord/src/monitor/reply-delivery.ts b/extensions/discord/src/monitor/reply-delivery.ts index 300e44567c8..c2dadca7a8c 100644 --- a/extensions/discord/src/monitor/reply-delivery.ts +++ b/extensions/discord/src/monitor/reply-delivery.ts @@ -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; }