From 99ff5b20f76603e68ab18559014f4107be58da05 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 8 Aug 2026 20:45:21 -0700 Subject: [PATCH] fix(discord): keep forum replies in their created thread (#120857) --- extensions/discord/src/outbound-payload.ts | 38 ++++- .../discord/src/send.creates-thread.test.ts | 150 ++++++++++++++++++ 2 files changed, 182 insertions(+), 6 deletions(-) diff --git a/extensions/discord/src/outbound-payload.ts b/extensions/discord/src/outbound-payload.ts index 06064a4d0732..7ce7c18a5ead 100644 --- a/extensions/discord/src/outbound-payload.ts +++ b/extensions/discord/src/outbound-payload.ts @@ -18,8 +18,9 @@ import { } from "./outbound-components.js"; import { createDiscordPayloadSendContext } from "./outbound-send-context.js"; import { hasDiscordMessageCreateAmbiguity } from "./retry.js"; -import { createDiscordSendReceipt } from "./send.receipt.js"; +import { createDiscordSendReceipt, createDiscordSendReceiptFromResults } from "./send.receipt.js"; import type { DiscordSendComponents, DiscordSendEmbeds } from "./send.shared.js"; +import type { DiscordSendResult } from "./send.types.js"; type DiscordOutboundPayloadContext = Parameters< NonNullable @@ -197,14 +198,39 @@ export async function sendDiscordOutboundPayload(params: { }); return attachChannelToResult("discord", result); } - return await sendTextMediaPayload({ + const payloadContext = { ...ctx, payload }; + const deliveredResults: DiscordSendResult[] = []; + let createdThreadId: string | undefined; + payloadContext.onDeliveryResult = async (result) => { + await ctx.onDeliveryResult?.(result); + const threadId = result.receipt?.threadId; + if (threadId && payloadContext.threadId == null) { + // A forum parent creates its conversation on the first platform send. + payloadContext.threadId = threadId; + createdThreadId = threadId; + } + if (createdThreadId && result.channelId && result.receipt) { + deliveredResults.push({ + messageId: result.messageId, + channelId: result.channelId, + receipt: result.receipt, + }); + } + }; + const result = await sendTextMediaPayload({ channel: "discord", - ctx: { - ...ctx, - payload, - }, + ctx: payloadContext, adapter: params.fallbackAdapter, }); + return createdThreadId + ? { + ...result, + receipt: createDiscordSendReceiptFromResults({ + results: deliveredResults, + threadId: createdThreadId, + }), + } + : result; } const result = await sendPayloadMediaSequenceOrFallback({ diff --git a/extensions/discord/src/send.creates-thread.test.ts b/extensions/discord/src/send.creates-thread.test.ts index f8bd77ffce35..32c057dd163c 100644 --- a/extensions/discord/src/send.creates-thread.test.ts +++ b/extensions/discord/src/send.creates-thread.test.ts @@ -14,6 +14,7 @@ vi.mock("openclaw/plugin-sdk/web-media", async () => { let addRoleDiscord: typeof import("./send.js").addRoleDiscord; let banMemberDiscord: typeof import("./send.js").banMemberDiscord; let createThreadDiscord: typeof import("./send.js").createThreadDiscord; +let discordOutbound: typeof import("./outbound-adapter.js").discordOutbound; let DiscordThreadInitialMessageError: typeof import("./send.js").DiscordThreadInitialMessageError; let listGuildEmojisDiscord: typeof import("./send.js").listGuildEmojisDiscord; let listThreadsDiscord: typeof import("./send.js").listThreadsDiscord; @@ -75,6 +76,64 @@ function timerDelayAt(source: MockCallSource, callIndex = 0) { return mockArg(source, callIndex, 1, `timer delay ${callIndex}`); } +function createDiscordForumPayloadHarness(parentType: ChannelType = ChannelType.GuildForum) { + const parentId = "700"; + const { rest, getMock, postMock } = makeDiscordRest(); + let threadCount = 0; + let messageCount = 0; + + getMock.mockImplementation(async (path: unknown) => { + const channelId = String(path).split("/").at(-1); + return { + id: channelId, + type: channelId === parentId ? parentType : ChannelType.PublicThread, + }; + }); + postMock.mockImplementation(async (path: unknown) => { + if (path === Routes.threads(parentId)) { + threadCount += 1; + const threadId = String(700 + threadCount); + return { + id: threadId, + message: { id: `starter-${threadCount}`, channel_id: threadId }, + }; + } + const channelId = String(path).split("/").at(-2); + messageCount += 1; + return { id: `message-${messageCount}`, channel_id: channelId }; + }); + + return { + parentId, + postMock, + run: async ( + payload: { text: string; mediaUrls?: string[] }, + options: { + threadId?: string; + onDeliveryResult?: Parameters< + NonNullable + >[0]["onDeliveryResult"]; + } = {}, + ) => + await discordOutbound.sendPayload?.({ + cfg: DISCORD_TEST_CFG, + to: `channel:${parentId}`, + text: payload.text, + payload, + ...(options.threadId ? { threadId: options.threadId } : {}), + ...(options.onDeliveryResult ? { onDeliveryResult: options.onDeliveryResult } : {}), + deps: { + discord: async (...[target, text, sendOptions]: Parameters) => + await sendMessageDiscord(target, text, { + ...sendOptions, + rest, + token: "t", + }), + }, + }), + }; +} + function createRateLimitError( response: Response, body: { message: string; retry_after: number; global: boolean }, @@ -110,6 +169,7 @@ beforeAll(async () => { uploadEmojiDiscord, uploadStickerDiscord, } = await import("./send.js")); + ({ discordOutbound } = await import("./outbound-adapter.js")); }); beforeEach(() => { @@ -126,6 +186,96 @@ afterAll(() => { }); describe("sendMessageDiscord", () => { + it.each([ + { + label: "a 2001-character reply", + payload: { text: "a".repeat(2001) }, + expectedThreadMessages: 1, + }, + { + label: "a reply with two image attachments", + payload: { + text: "Generated images", + mediaUrls: ["https://example.com/first.jpg", "https://example.com/second.jpg"], + }, + expectedThreadMessages: 2, + }, + ])("keeps $label in one automatically created forum thread", async (testCase) => { + const { parentId, postMock, run } = createDiscordForumPayloadHarness(); + const onDeliveryResult = vi.fn(); + + const result = await run(testCase.payload, { onDeliveryResult }); + + const requestPaths = postMock.mock.calls.map((call) => call[0]); + expect(requestPaths).toEqual([ + Routes.threads(parentId), + ...Array.from({ length: testCase.expectedThreadMessages }, () => + Routes.channelMessages("701"), + ), + ]); + expect(onDeliveryResult.mock.calls.map(([delivery]) => delivery.channelId)).toEqual( + Array.from({ length: testCase.expectedThreadMessages + 1 }, () => "701"), + ); + expect(result?.receipt).toMatchObject({ + threadId: "701", + platformMessageIds: [ + "starter-1", + ...Array.from( + { length: testCase.expectedThreadMessages }, + (_, index) => `message-${index + 1}`, + ), + ], + }); + }); + + it("keeps chunked regular-channel replies on their original channel", async () => { + const { parentId, postMock, run } = createDiscordForumPayloadHarness(ChannelType.GuildText); + + const result = await run({ text: "a".repeat(2001) }); + + expect(postMock.mock.calls.map((call) => call[0])).toEqual([ + Routes.channelMessages(parentId), + Routes.channelMessages(parentId), + ]); + expect(result?.receipt?.threadId).toBeUndefined(); + expect(result?.receipt?.platformMessageIds).toEqual(["message-2"]); + }); + + it("keeps chunked replies targeted at an explicitly selected thread", async () => { + const { postMock, run } = createDiscordForumPayloadHarness(); + + const result = await run({ text: "a".repeat(2001) }, { threadId: "701" }); + + expect(postMock.mock.calls.map((call) => call[0])).toEqual([ + Routes.channelMessages("701"), + Routes.channelMessages("701"), + ]); + expect(result?.receipt?.threadId).toBeUndefined(); + expect(result?.receipt?.platformMessageIds).toEqual(["message-2"]); + }); + + it("does not attempt a follow-up when forum thread creation is rejected", async () => { + const { parentId, postMock, run } = createDiscordForumPayloadHarness(); + postMock.mockRejectedValueOnce(new Error("missing access")); + + await expect(run({ text: "a".repeat(2001) })).rejects.toThrow("missing access"); + + expect(postMock).toHaveBeenCalledOnce(); + expect(postMock.mock.calls[0]?.[0]).toBe(Routes.threads(parentId)); + }); + + it("does not send a forum follow-up when delivery bookkeeping rejects the starter", async () => { + const { parentId, postMock, run } = createDiscordForumPayloadHarness(); + const onDeliveryResult = vi.fn().mockRejectedValue(new Error("delivery bookkeeping failed")); + + await expect(run({ text: "a".repeat(2001) }, { onDeliveryResult })).rejects.toThrow( + "delivery bookkeeping failed", + ); + + expect(onDeliveryResult).toHaveBeenCalledOnce(); + expect(postMock.mock.calls.map((call) => call[0])).toEqual([Routes.threads(parentId)]); + }); + it("creates a thread", async () => { const { rest, getMock, postMock } = makeDiscordRest(); postMock.mockResolvedValue({ id: "t1" });