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,