From d7b51668624393d26053382e87cd9b73a4233dec Mon Sep 17 00:00:00 2001 From: Codex Date: Wed, 3 Jun 2026 22:18:26 +0900 Subject: [PATCH] fix(discord): route >2000-char finals to chunked send, keep editMessage pure editMessage is called repeatedly by the progress ticker and dashboard refresh, so appending overflow chunks there duplicated text on every edit. Revert editMessage to a pure single-message edit and instead expose the platform per-message cap via Channel.maxMessageLength; deliverOpenWorkItem now skips the doomed in-place edit and hands the full payload to the chunking sendMessage path when the final exceeds that cap. Co-Authored-By: Claude Opus 4 --- src/channels/discord.test.ts | 24 ++++---- src/channels/discord.ts | 30 ++++----- src/message-runtime-delivery.test.ts | 91 ++++++++++++++++++++++++++++ src/message-runtime-delivery.ts | 16 ++++- src/types.ts | 4 ++ 5 files changed, 134 insertions(+), 31 deletions(-) create mode 100644 src/message-runtime-delivery.test.ts diff --git a/src/channels/discord.test.ts b/src/channels/discord.test.ts index 82713e6..431f2d5 100644 --- a/src/channels/discord.test.ts +++ b/src/channels/discord.test.ts @@ -960,7 +960,12 @@ describe('DiscordChannel', () => { expect(mockChannel.send).not.toHaveBeenCalled(); }); - it('edits first chunk and appends the rest when text exceeds 2000 chars', async () => { + it('stays a pure single-message edit and never appends follow-up messages', async () => { + // editMessage is called repeatedly by the progress ticker and dashboard + // refresh; it must only ever edit the one tracked message. Splitting long + // text into extra messages here would duplicate the overflow on every + // repeated edit. Long final payloads are routed to sendMessage by the + // delivery layer instead (see message-runtime-delivery). const opts = createTestOpts(); const channel = new DiscordChannel('test-token', opts); await channel.connect(); @@ -975,18 +980,13 @@ describe('DiscordChannel', () => { const longText = 'x'.repeat(3000); await channel.editMessage('dc:1234567890123456', 'msg_1', longText); - // First chunk goes into the existing (tracked) message via edit. expect(msg.edit).toHaveBeenCalledTimes(1); - expect((msg.edit.mock.calls[0][0] as string).length).toBeLessThanOrEqual( - 2000, - ); - // The overflow is appended as a follow-up message (one message cannot - // hold >2000 chars), so the edit never sends an oversized body. - expect(mockChannel.send).toHaveBeenCalledTimes(1); - expect(mockChannel.send).toHaveBeenCalledWith({ - content: 'x'.repeat(1000), - flags: 1 << 2, - }); + expect(mockChannel.send).not.toHaveBeenCalled(); + }); + + it('advertises the Discord per-message length limit', () => { + const channel = new DiscordChannel('test-token', createTestOpts()); + expect(channel.maxMessageLength).toBe(2000); }); }); diff --git a/src/channels/discord.ts b/src/channels/discord.ts index 1c83e45..8cb49d7 100644 --- a/src/channels/discord.ts +++ b/src/channels/discord.ts @@ -44,6 +44,10 @@ const MD_LINK_RE = /\[[^\]]*\]\((\/[^)]+)\)/g; // Captures the fence marker (``` or ```` etc.) and the optional language tag. const FENCE_LINE_RE = /^(`{3,})([^\s`]*)\s*$/; +// Discord rejects any message/edit body longer than 2000 characters with +// "Invalid Form Body … Must be 2000 or fewer in length". +export const DISCORD_MAX_MESSAGE_LENGTH = 2000; + /** * Split `text` into chunks ≤ `maxLength` chars while keeping every chunk a * valid standalone Markdown snippet for Discord. If a chunk has to end inside @@ -335,6 +339,7 @@ export interface DiscordChannelOpts { export class DiscordChannel implements Channel { name = 'discord'; + readonly maxMessageLength = DISCORD_MAX_MESSAGE_LENGTH; private client: Client | null = null; private opts: DiscordChannelOpts; @@ -629,7 +634,7 @@ export class DiscordChannel implements Channel { cleaned = options.rawMarkdown ? cleaned : formatOutbound(cleaned); // Discord has a 2000 character limit per message and 10 attachments per message - const MAX_LENGTH = 2000; + const MAX_LENGTH = DISCORD_MAX_MESSAGE_LENGTH; const MAX_ATTACHMENTS = 10; const sentMessageIds: string[] = []; let chunkCount = 0; @@ -948,23 +953,12 @@ export class DiscordChannel implements Channel { // Dashboard callers opt out with rawMarkdown: true to preserve // intentional bold/italic section headers. const cleaned = options.rawMarkdown ? text : formatOutbound(text); - // Discord rejects edits/messages longer than 2000 chars with - // "Invalid Form Body … Must be 2000 or fewer". A single message cannot - // hold the overflow, so edit the tracked message with the first chunk - // and append the remaining chunks as follow-up messages. - const MAX_LENGTH = 2000; - if (cleaned.length <= MAX_LENGTH) { - await msg.edit(cleaned); - } else { - const chunks = chunkForDiscord(cleaned, MAX_LENGTH); - await msg.edit(chunks[0]); - for (let i = 1; i < chunks.length; i++) { - await (channel as TextChannel).send({ - content: chunks[i], - flags: MessageFlags.SuppressEmbeds, - }); - } - } + // editMessage only ever edits a single message. Callers that may produce + // text longer than DISCORD_MAX_MESSAGE_LENGTH (e.g. final result + // delivery) must route to sendMessage instead; this method must stay a + // pure single-message edit so repeated edits (progress ticker, dashboard + // refresh) never spawn extra follow-up messages. + await msg.edit(cleaned); logger.info( { jid, diff --git a/src/message-runtime-delivery.test.ts b/src/message-runtime-delivery.test.ts new file mode 100644 index 0000000..7b4b2c5 --- /dev/null +++ b/src/message-runtime-delivery.test.ts @@ -0,0 +1,91 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +vi.mock('./db.js', () => ({ + markWorkItemDelivered: vi.fn(), + markWorkItemDeliveryRetry: vi.fn(), +})); + +import { deliverOpenWorkItem } from './message-runtime-delivery.js'; +import type { Channel } from './types.js'; + +const silentLog = { info: vi.fn(), warn: vi.fn(), error: vi.fn() }; + +function makeChannel(overrides: Partial = {}): Channel { + return { + name: 'discord-owner', + maxMessageLength: 2000, + connect: vi.fn(), + isConnected: () => true, + ownsJid: () => true, + disconnect: vi.fn(), + sendMessage: vi.fn().mockResolvedValue(undefined), + editMessage: vi.fn().mockResolvedValue(undefined), + ...overrides, + } as unknown as Channel; +} + +function makeItem(resultPayload: string) { + return { + id: 'wi_1', + chat_jid: 'dc:123', + result_payload: resultPayload, + delivery_role: 'owner' as const, + delivery_attempts: 0, + attachments: [], + } as any; +} + +describe('deliverOpenWorkItem replaceMessageId routing', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('edits the tracked message in place when the payload fits one message', async () => { + const channel = makeChannel(); + await deliverOpenWorkItem({ + channel, + item: makeItem('short final'), + log: silentLog, + replaceMessageId: 'msg_1', + isDuplicateOfLastBotFinal: () => false, + openContinuation: vi.fn(), + }); + + expect(channel.editMessage).toHaveBeenCalledTimes(1); + expect(channel.sendMessage).not.toHaveBeenCalled(); + }); + + it('skips the edit and sends a new (chunkable) message when the payload exceeds the channel limit', async () => { + const channel = makeChannel(); + const longText = 'x'.repeat(3000); + await deliverOpenWorkItem({ + channel, + item: makeItem(longText), + log: silentLog, + replaceMessageId: 'msg_1', + isDuplicateOfLastBotFinal: () => false, + openContinuation: vi.fn(), + }); + + // Must NOT attempt the in-place edit (Discord would reject >2000 chars). + expect(channel.editMessage).not.toHaveBeenCalled(); + // Full payload handed to sendMessage, which performs the chunking. + expect(channel.sendMessage).toHaveBeenCalledTimes(1); + expect((channel.sendMessage as any).mock.calls[0][1]).toBe(longText); + }); + + it('edits in place for long payloads when the channel declares no length limit', async () => { + const channel = makeChannel({ maxMessageLength: undefined }); + await deliverOpenWorkItem({ + channel, + item: makeItem('y'.repeat(3000)), + log: silentLog, + replaceMessageId: 'msg_1', + isDuplicateOfLastBotFinal: () => false, + openContinuation: vi.fn(), + }); + + expect(channel.editMessage).toHaveBeenCalledTimes(1); + expect(channel.sendMessage).not.toHaveBeenCalled(); + }); +}); diff --git a/src/message-runtime-delivery.ts b/src/message-runtime-delivery.ts index 4d01962..d67671e 100644 --- a/src/message-runtime-delivery.ts +++ b/src/message-runtime-delivery.ts @@ -54,8 +54,22 @@ export async function deliverOpenWorkItem(args: { return true; } + // editMessage replaces a single tracked message in place. If the final + // payload is longer than the channel's per-message limit it cannot fit in + // one edited message, so skip the edit path and fall through to a chunked + // sendMessage instead of attempting an edit the platform would reject + // (e.g. Discord "Invalid Form Body … Must be 2000 or fewer in length"). + const maxLen = args.channel.maxMessageLength; + const payloadFitsSingleMessage = + maxLen == null || args.item.result_payload.length <= maxLen; + try { - if (replaceMessageId && args.channel.editMessage && !hasAttachments) { + if ( + replaceMessageId && + args.channel.editMessage && + !hasAttachments && + payloadFitsSingleMessage + ) { args.log.info( buildDeliveryLogContext(args.channel, args.item, { deliveryAttempts: args.item.delivery_attempts + 1, diff --git a/src/types.ts b/src/types.ts index 849652f..7367526 100644 --- a/src/types.ts +++ b/src/types.ts @@ -274,6 +274,10 @@ export interface ChannelOutboundAuditMeta { export interface Channel { name: string; + // Optional: max characters the platform accepts in a single message/edit. + // Callers delivering long text use this to decide between an in-place edit + // and a chunked sendMessage. Undefined means "no known hard limit". + maxMessageLength?: number; connect(): Promise; sendMessage( jid: string,