diff --git a/changelog.d/fix-discord-history-keeps-host-blocks.fixed.md b/changelog.d/fix-discord-history-keeps-host-blocks.fixed.md new file mode 100644 index 000000000..28a98f2d3 --- /dev/null +++ b/changelog.d/fix-discord-history-keeps-host-blocks.fixed.md @@ -0,0 +1,7 @@ +- The Discord module's edits and history sync no longer erase what the host added to a stored message. A message is now stored with its own text first, `''` when it has none, followed by its images, inlined file text and attachment notes, and its row's metadata carries `discordTextFirst: true`. An edit, live or found by the history sync, replaces that first block alone. Before, an edit rewrote the message as Discord's plain text, dropping its images, attachment notes and inlined file text. The history sync, which runs when the client becomes ready, did the same to any message with a note or an inlined file, since it compared Discord's text with all of the stored text at once, and a message whose only content was such a note or file then read as nothing at all. An update that leaves the text as it was now changes nothing, and an edit that removes a message's text, leaving its attachment, now reaches a message stored from now on. A live edit leaves a row stored before this change alone unless Discord's text is there and differs from the row's first block. It then puts the text in that block's place when the block is text, and before it when it isn't, and keeps the rest. Before, it rewrote the row as the text alone. A message that had no text of its own and was stored beginning with a file's contents or a note loses that block, where the rewrite lost everything, until the history sync stores it again. +- A live edit or delete now reaches a message that arrived live. Both looked the row up in the module context's external-id index, which holds only rows a module stored through its context, such as the history sync's, so they did nothing for any other message. +- A message the history sync missed is now stored as the live path stores it, with its images, small text files and attachment notes, where only its text was stored before. The sync also stores a row again through the live conversion when it doesn't hold what that conversion gives its message now: an attachment-only message an earlier sync blanked reads again, one it stripped of its notes or files gets them back, and an attachment removed from the message goes. One whose text changed while the client was away keeps its files. A row stored before this change for a message without text, whose first block is text, is stored again once, since that block may be a caption the message lost while the client was away. A rebuild whose text file can't be fetched writes nothing over a row holding other text, which may be that file's, so the row keeps what it holds and the next sync tries again; a row missing its file gets the notice that it wasn't fetched. A row the live path stored before the sync began is neither rewritten nor fetched again; one it stores while the sync fetches history may have its attachments fetched again, and is then left as the live path wrote it. +- The history sync now leaves alone a message the live path stored, changed or was told was deleted while it ran; an update that changed nothing doesn't count. It runs the conversions it needs first, then reads the store again and writes with no await between. A message the live path stored while it ran, which its fetch couldn't hold, is no longer removed as deleted. A row another writer removed while it ran isn't stored again. Before, a live edit made while the sync fetched history was written over with the fetch's older text. It writes nothing once the module has stopped or started again. A message a context strategy stored as a body, in shards, is kept as stored by the sync and by a live edit, since a body can't be edited in place, so an edit to it isn't taken in, and a live one writes a console line saying so. Before, the sync threw on a shard and stopped at it on every pass, and a live edit that found a shard threw too. +- `fetch_history` lists each message's attachments by filename, type and size, without their URLs, which work for anyone holding them until they expire. +- A send that posted its message but whose thread wasn't started now counts that message as the one the bot last sent, so the agent's thoughts are edited into it. Before, the message wasn't recorded, and the thoughts went into the message the bot sent before it, or into none. The send's result is the same error as before. +- Host authors: a Discord message stored from now on has its own text as its first content block, `''` when it has none; a row stored before keeps its layout until the history sync stores it again. The request builder drops an empty text block, as it already did, but a UI that renders stored content should expect one, and the history module's `raw` format shows it. `HistoryMessage` gains an optional `attachments`, which a custom `DiscordClientInterface` can fill; left out, the sync can't compare a message's attachments, so it rebuilds no row, though it still changes a row's text as a live edit does. `MessageQuery` gains `externalIds`, which context-manager already accepted. A `DiscordClientInterface`'s `sendMessage` may resolve with `threadFailure`, why the thread it was asked for wasn't started; one that throws instead works as before, and the message it sent isn't counted as the bot's last. diff --git a/src/modules/discord/discord-js-client.ts b/src/modules/discord/discord-js-client.ts index 4111a2141..d24c99225 100644 --- a/src/modules/discord/discord-js-client.ts +++ b/src/modules/discord/discord-js-client.ts @@ -29,6 +29,17 @@ export interface DiscordJsClientConfig { channelIds?: string[]; } +/** An attachment as both a live message and history carry it. */ +function toAttachment(a: Attachment): DiscordAttachment { + return { + id: a.id, + filename: a.name ?? 'unknown', + url: a.url, + contentType: a.contentType ?? undefined, + size: a.size, + }; +} + export class DiscordJsClient implements DiscordClientInterface { private client: Client; private token: string; @@ -68,7 +79,11 @@ export class DiscordJsClient implements DiscordClientInterface { }); this.client.on('messageUpdate', (_oldMessage, newMessage) => { - if (!newMessage.content || !this.editHandler) return; + // content is null for a partial message, one discord.js hadn't cached + // and whose update carried no text, so there's no text to compare; a + // cached message keeps the text it had. An edit that leaves no text, + // beside an attachment, is '' and is still an edit. + if (typeof newMessage.content !== 'string' || !this.editHandler) return; this.editHandler(newMessage.id, newMessage.content); }); @@ -114,13 +129,7 @@ export class DiscordJsClient implements DiscordClientInterface { } private convertMessage(message: Message): DiscordMessageData { - const attachments: DiscordAttachment[] = message.attachments.map((a: Attachment) => ({ - id: a.id, - filename: a.name ?? 'unknown', - url: a.url, - contentType: a.contentType ?? undefined, - size: a.size, - })); + const attachments = message.attachments.map(toAttachment); // Extract mentioned user IDs const mentionedUserIds = message.mentions.users.map((u: User) => u.id); @@ -160,7 +169,7 @@ export class DiscordJsClient implements DiscordClientInterface { channelId: string, content: string, options?: { replyTo?: string; createThread?: { name: string } } - ): Promise<{ messageId: string }> { + ): Promise<{ messageId: string; threadFailure?: string }> { const channel = await this.client.channels.fetch(channelId); if (!channel || !('send' in channel)) { throw new Error(`Channel ${channelId} not found or not a text channel`); @@ -173,7 +182,13 @@ export class DiscordJsClient implements DiscordClientInterface { }); if (options?.createThread && 'threads' in textChannel) { - await message.startThread({ name: options.createThread.name }); + try { + await message.startThread({ name: options.createThread.name }); + } catch (error) { + // The message is out, so the caller counts it as sent before it + // reports why its thread wasn't started. + return { messageId: message.id, threadFailure: error instanceof Error ? error.message : String(error) }; + } } return { messageId: message.id }; @@ -341,6 +356,7 @@ export class DiscordJsClient implements DiscordClientInterface { authorName: m.author.username, isBot: m.author.bot, content: m.content, + attachments: m.attachments.map(toAttachment), timestamp: m.createdAt, replyTo: m.reference ? { diff --git a/src/modules/discord/index.ts b/src/modules/discord/index.ts index 5d2571213..acf80b836 100644 --- a/src/modules/discord/index.ts +++ b/src/modules/discord/index.ts @@ -8,6 +8,7 @@ */ import type { ContentBlock } from '@animalabs/membrane'; +import type { StoredMessage } from '@animalabs/context-manager'; import type { Module, ModuleContext, @@ -24,11 +25,13 @@ import type { DiscordModuleConfig, DiscordModuleState, ConversationContext, + DiscordAttachment, DiscordClientInterface, DiscordMessageData, DiscordMessageEvent, DiscordEditEvent, DiscordDeleteEvent, + HistoryMessage, SendInput, SendDMInput, ReplyInput, @@ -98,10 +101,126 @@ const CONVERSATION_TIMEOUT_MS = 30 * 60 * 1000; // 30 minutes /** What the agent reads for a text attachment whose contents couldn't be * fetched: by an error, or by an answer that wasn't OK. */ -function attachmentNotFetched(filename: string): ContentBlock { +function attachmentNotFetched(filename: string): { type: 'text'; text: string } { return { type: 'text', text: `[Attachment: ${filename} (failed to fetch)]` }; } +/** What a stored message reads when it has no text of its own and nothing the + * host added, so the agent still sees that a message was there. */ +const EMPTY_MESSAGE = '[Empty message]'; + +/** + * Metadata on a stored Discord message whose first block is its own text, + * '' when it has none, and every block after it the host's: images, inlined + * file text, attachment notes and EMPTY_MESSAGE. An edit replaces content and + * can't touch metadata, so the mark holds for the row's life, and it tells an + * edit or a sync which block is Discord's to replace. For the same reason a + * row stored before the mark never gets it, even once a history sync + * rebuilds it in this layout. The request builder drops an empty text block, + * as it does wherever one occurs. + */ +const TEXT_FIRST = 'discordTextFirst'; + +/** A message's own text as a stored row holds it: '' when it's blank. */ +function ownText(content: string): string { + return content.trim() ? content : ''; +} + +const isEmptyMarker = (block: ContentBlock): boolean => block.type === 'text' && block.text === EMPTY_MESSAGE; +const isBlankText = (block: ContentBlock): boolean => block.type === 'text' && !block.text.trim(); + +/** + * A text-first row's content: the message's own text, '' when it has none, + * then `after`, what the host added; EMPTY_MESSAGE when there's neither. + */ +function textFirst(text: string, after: ContentBlock[] = []): ContentBlock[] { + return [{ type: 'text', text }, ...(text || after.length > 0 ? after : [{ type: 'text' as const, text: EMPTY_MESSAGE }])]; +} + +/** A text-first row's content with `text` as the message's own text, in + * place of its first block, keeping what the host added after it. */ +function withOwnText(content: ContentBlock[], text: string): ContentBlock[] { + return textFirst(ownText(text), content.slice(1).filter((block) => !isEmptyMarker(block))); +} + +/** + * The content a stored Discord message should hold once Discord's text for it + * is `discordText`, or null when it holds that already. + * + * A text-first row keeps every block the host added and replaces only its own + * text; EMPTY_MESSAGE goes once there is text, and comes back when nothing is + * left. A row stored before that layout held its own text first when it had + * any, but a message without text began with what the host added, which may + * be text, a file's contents or a note, so its first block can't be told + * apart. Such a row is left alone unless Discord's text is there and differs + * from its first block, and then that text takes the first block's place + * when it's text, and goes before it when it isn't, and EMPTY_MESSAGE goes. + * So a message without text, stored beginning with a file's contents or a + * note, loses that block. The history sync, which has the message's + * attachments, rebuilds such a row instead (needsRebuild). + */ +function contentForText(stored: StoredMessage, discordText: string): ContentBlock[] | null { + const text = ownText(discordText); + const [first, ...rest] = stored.content; + if (stored.metadata?.[TEXT_FIRST] === true) { + if (first?.type === 'text' && first.text === text) return null; + return withOwnText(stored.content, text); + } + if (!text || (first?.type === 'text' && first.text === discordText)) return null; + return [{ type: 'text', text: discordText }, ...(first?.type === 'text' ? rest : stored.content).filter((block) => !isEmptyMarker(block))]; +} + +/** + * Whether a stored row lacks or holds more than the live conversion gives + * Discord's `message` now, its own text aside, so the history sync stores it + * through that conversion again. The conversion gives each attachment one + * block: an image, a file's text, or a note when it isn't fetched or isn't + * read. So a text-first row holds one block per attachment after its text, + * EMPTY_MESSAGE aside, which the text's rule keeps. A row stored before that + * layout held its text, when it had any, then those blocks, or EMPTY_MESSAGE + * alone; one that doesn't was blanked or stripped by an earlier edit or sync, + * or its message gained or lost text, or lost an attachment. Its first block, + * when it is the message's own text, counts as that text, even when the text + * reads as EMPTY_MESSAGE. For a message without text of its own, a first + * block that's text may be what the host added or a caption the message has + * since lost, which no count tells apart, so that row is stored again; it + * then begins with its own '' text. One whose text alone changed is left to + * the text's rule, which keeps the blocks after it. History without + * attachments can't be compared, so it rebuilds nothing. + */ +function needsRebuild(stored: StoredMessage, message: HistoryMessage): boolean { + if (message.attachments === undefined) return false; + const attachments = message.attachments.length; + const held = (blocks: ContentBlock[]) => blocks.filter((block) => !isBlankText(block) && !isEmptyMarker(block)).length; + if (stored.metadata?.[TEXT_FIRST] === true) return held(stored.content.slice(1)) !== attachments; + const own = ownText(message.content); + const [first, ...rest] = stored.content; + if (!own && first?.type === 'text' && !isBlankText(first) && !isEmptyMarker(first)) return true; + const expected = (own ? 1 : 0) + attachments; + const counted = own && first?.type === 'text' && first.text === own ? 1 + held(rest) : held(stored.content); + return counted !== expected || (expected === 0 && !stored.content.some(isEmptyMarker)); +} + +/** + * Whether a row holds non-blank text besides its message's own text now, + * which may be what a file gave. A marked row whose message's text changed + * counts its old text here too, which can delay its rebuild until a fetch + * succeeds but never loses text. + */ +function holdsOtherText(stored: StoredMessage, message: HistoryMessage): boolean { + const own = ownText(message.content); + const [first, ...rest] = stored.content; + const others = own && first?.type === 'text' && first.text === own ? rest : stored.content; + return others.some((block) => block.type === 'text' && !isBlankText(block) && !isEmptyMarker(block)); +} + +/** Whether a message's conversion holds, after the message's own text, the + * note for an attachment of its whose text wasn't fetched. */ +function holdsNotFetched(content: ContentBlock[], message: HistoryMessage): boolean { + const notes = new Set((message.attachments ?? []).map((attachment) => attachmentNotFetched(attachment.filename).text)); + return content.slice(1).some((block) => block.type === 'text' && notes.has(block.text)); +} + /** * Discord module for the agent framework. */ @@ -125,6 +244,12 @@ export class DiscordModule implements Module { // Last message sent by a tool (for thought-editing) private lastSentMessage: { channelId: string; messageId: string } | null = null; + // One set per running history sync: the Discord messages the live path + // stored, edited or was told were deleted since that sync began. What the + // live path saw is newer than what the sync fetched, so the sync leaves + // those messages alone. + private readonly liveDuringSync = new Set>(); + // Typing keepalive: agentName -> { interval, deadline }. Started on // inference:started, stopped on the turn's terminal trace event. Discord's // typing indicator auto-expires after ~10s, so we re-trigger on an interval @@ -413,7 +538,7 @@ export class DiscordModule implements Module { }, { name: 'fetch_history', - description: 'Fetch recent message history from a channel', + description: 'Fetch recent message history from a channel. A message with attachments lists each one\'s filename, type and size; their contents aren\'t included.', inputSchema: { type: 'object', properties: { @@ -516,8 +641,10 @@ export class DiscordModule implements Module { }); } - // Build content blocks from message and attachments - const contentBlocks = msg.contentBlocks ?? [{ type: 'text' as const, text: msg.content }]; + // Built from the message and its attachments, or from its text alone + // when an event carries no blocks; text first, as TEXT_FIRST marks. + const contentBlocks = msg.contentBlocks ?? textFirst(ownText(msg.content)); + this.noteLive(msg.discordMessageId); // Return declarative response - framework applies all writes atomically return { @@ -529,6 +656,7 @@ export class DiscordModule implements Module { external: { source: 'discord', id: msg.discordMessageId }, channelId: msg.channelId, timestamp: msg.timestamp, + [TEXT_FIRST]: true, }, }, ], @@ -540,29 +668,41 @@ export class DiscordModule implements Module { // Handle Discord message edit if (event.type === 'discord:edit') { const edit = event as DiscordEditEvent; - const internalId = state.findMessageByExternalId('discord', edit.discordMessageId); - if (!internalId) { + // The stored row, not only its id: an edit changes the message's own + // text and keeps what the host added beside it. + // An edit to a message nobody has stored yet is dropped: whatever + // stores the message, its live add or a history sync running now, + // stores the text it got, and the next sync's text rule changes it. + // A row stored in shards can't be edited one shard at a time, so it + // keeps its text, and the host's log says so. Only an edit that writes + // marks the message live, so an update that changes nothing leaves a + // running sync free to rewrite the row. + const stored = this.storedRow(edit.discordMessageId); + if (!stored) return {}; + if (stored.bodyGroupId) { + console.warn(`Discord: An update to message ${edit.discordMessageId} wasn't applied: it's stored in shards, which can't be edited, so if its text changed, the agent still has the earlier text`); return {}; } - return { - editMessages: [ - { - messageId: internalId, - content: [{ type: 'text', text: edit.newContent }], - }, - ], - }; + const content = contentForText(stored, edit.newContent); + if (!content) return {}; + this.noteLive(edit.discordMessageId); + return { editMessages: [{ messageId: stored.id, content }] }; } // Handle Discord message delete if (event.type === 'discord:delete') { const del = event as DiscordDeleteEvent; - const internalId = state.findMessageByExternalId('discord', del.discordMessageId); - if (!internalId) { + // A row stored in shards is removed whole: the framework takes out the + // body a shard belongs to. The message is the live path's even when no + // row is found, so a running sync doesn't store what its fetch may + // still hold. + this.noteLive(del.discordMessageId); + const stored = this.storedRow(del.discordMessageId); + if (!stored) { return {}; } return { - removeMessages: [internalId], + removeMessages: [stored.id], }; } @@ -731,131 +871,176 @@ export class DiscordModule implements Module { /** * Sync history for a specific channel - compare with Context Manager and update. * Uses queryMessages() to enumerate stored messages for proper edit/delete detection. + * + * The live path can store, edit or remove a row whenever the sync awaits. + * So the sync fetches history and runs the live conversion it needs first, + * then reads the store again and writes with no await between that read + * and its writes. It leaves alone any message the live path stored, + * changed or was told was deleted while it ran (liveDuringSync), an update + * that changed nothing aside. A live edit to a message nobody had stored + * yet is dropped, so a message the sync stores holds the text it fetched + * until the next sync's text rule changes it. The framework writes an + * event's response only once every module has processed the event, so an + * event processed before the sync began can still be written after the + * sync's own writes, which this doesn't cover. */ private async syncChannelHistory(channelId: string): Promise<{ newMessages: number; editedMessages: number; deletedMessages: number; }> { - if (!this.ctx) { - return { newMessages: 0, editedMessages: 0, deletedMessages: 0 }; + const unchanged = { newMessages: 0, editedMessages: 0, deletedMessages: 0 }; + const ctx = this.ctx; + if (!ctx) { + return unchanged; } - - const scrollback = this.config.historyScrollback ?? 50; - - // Fetch recent history from Discord - const discordMessages = await this.client.fetchHistory(channelId, { limit: scrollback }); - - // Build a map of Discord message ID -> content for comparison - const discordMessageMap = new Map(); - for (const msg of discordMessages) { - discordMessageMap.set(msg.id, { - content: msg.content, - authorName: msg.authorName, - timestamp: msg.timestamp, - }); - } - - // Query Context Manager for all Discord messages from this channel - const { messages: storedMessages } = this.ctx.queryMessages({ - source: 'discord', - metadata: { channelId }, - }); - - // Build a map of stored message external IDs -> internal data - const storedMessageMap = new Map(); - for (const msg of storedMessages) { - const external = msg.metadata?.external as { id?: string } | undefined; - if (external?.id) { - // Extract text content for comparison - const textContent = msg.content - .filter((block): block is { type: 'text'; text: string } => block.type === 'text') - .map((block) => block.text) - .join('\n'); - storedMessageMap.set(external.id, { - internalId: msg.id, - content: textContent, - sourceTimestamp: typeof msg.metadata?.timestamp === 'number' - ? msg.metadata.timestamp - : undefined, - }); + const live = new Set(); + this.liveDuringSync.add(live); + try { + const scrollback = this.config.historyScrollback ?? 50; + // The rows stored as the sync begins. One that is gone by the time it + // writes was removed meanwhile, and isn't the sync's to bring back. + const before = this.storedByDiscordId(ctx, channelId); + + // Fetch recent history from Discord. If the module stopped or restarted + // meanwhile, this sync's context is gone and it writes nothing. + const discordMessages = await this.client.fetchHistory(channelId, { limit: scrollback }); + if (this.ctx !== ctx) return unchanged; + + // Discord's ids, for delete detection inside the fetched window. + const discordIds = new Set(discordMessages.map((msg) => msg.id)); + + // Discord history is newest-first, while addMessage appends to the stored + // conversation. Process oldest-first so missed messages retain chronology. + const chronologicalMessages = [...discordMessages].reverse(); + + // The live conversion, which fetches attachments, for a message the + // store lacks and for a row that doesn't hold what the conversion gives + // the message now (needsRebuild). A row stored in shards can't be + // edited one shard at a time, so it's left as it is. + const converted = new Map(); + for (const discordMsg of chronologicalMessages) { + const stored = before.get(discordMsg.id); + if (stored && (stored.bodyGroupId || !needsRebuild(stored, discordMsg))) continue; + const content = await this.convertMessageToContent(discordMsg); + if (this.ctx !== ctx) return unchanged; + // A failed fetch never costs a row the text it holds: when a file + // wasn't fetched, a row holding other text, which may be that file's, + // keeps what it holds, and the next sync tries again. + if (stored && holdsNotFetched(content, discordMsg) && holdsOtherText(stored, discordMsg)) continue; + converted.set(discordMsg.id, content); } - } - let newMessages = 0; - let editedMessages = 0; - let deletedMessages = 0; - - // Discord history is newest-first, while addMessage appends to the stored - // conversation. Process oldest-first so missed messages retain chronology. - const chronologicalMessages = [...discordMessages].reverse(); - for (const discordMsg of chronologicalMessages) { - const stored = storedMessageMap.get(discordMsg.id); - - if (!stored) { - // New message we haven't seen - // For bot's own messages (from previous sessions), use the agent name - // This ensures continuity of conversation history - const isBotMessage = discordMsg.authorId === this.state.botUserId; - const agentName = this.ctx.getAgents()[0]?.name; - const participantName = isBotMessage - ? (agentName ?? discordMsg.authorName) - : discordMsg.authorName; - - this.ctx.addMessage( - participantName, - [{ type: 'text', text: discordMsg.content }], - { - external: { source: 'discord', id: discordMsg.id }, - channelId, - timestamp: discordMsg.timestamp.getTime(), - isOwnMessage: isBotMessage, // Mark as our own for reference + // From here on nothing awaits, so the rows read now are the rows written. + const storedMessageMap = this.storedByDiscordId(ctx, channelId); + let newMessages = 0; + let editedMessages = 0; + let deletedMessages = 0; + for (const discordMsg of chronologicalMessages) { + if (live.has(discordMsg.id)) continue; + const stored = storedMessageMap.get(discordMsg.id); + + if (!stored) { + // A message we haven't stored. One that was stored when the sync + // began, and isn't now, was removed meanwhile: not the sync's to + // bring back. + const content = converted.get(discordMsg.id); + if (!content || before.has(discordMsg.id)) continue; + // For bot's own messages (from previous sessions), use the agent name + // This ensures continuity of conversation history + const isBotMessage = discordMsg.authorId === this.state.botUserId; + const agentName = ctx.getAgents()[0]?.name; + const participantName = isBotMessage + ? (agentName ?? discordMsg.authorName) + : discordMsg.authorName; + ctx.addMessage( + participantName, + content, + { + external: { source: 'discord', id: discordMsg.id }, + channelId, + timestamp: discordMsg.timestamp.getTime(), + isOwnMessage: isBotMessage, // Mark as our own for reference + [TEXT_FIRST]: true, + } + ); + newMessages++; + } else if (!stored.bodyGroupId) { + // We have this message - check whether it changed (offline edit detection) + const content = needsRebuild(stored, discordMsg) + ? converted.get(discordMsg.id) + : contentForText(stored, discordMsg.content); + if (content) { + ctx.editMessage(stored.id, content); + editedMessages++; } + } + } + + // Check for deletes only inside the history we actually fetched. A short + // response covers complete available history; a full page has an older + // boundary whose tied millisecond must be preserved. + const fetchedCompleteHistory = discordMessages.length < scrollback; + const oldestFetchedTimestamp = discordMessages.length > 0 + ? Math.min(...discordMessages.map((msg) => msg.timestamp.getTime())) + : undefined; + for (const [externalId, stored] of storedMessageMap) { + const sourceTimestamp = typeof stored.metadata?.timestamp === 'number' + ? stored.metadata.timestamp + : undefined; + const isInsideFetchedWindow = fetchedCompleteHistory || ( + sourceTimestamp !== undefined && + oldestFetchedTimestamp !== undefined && + // Discord timestamps are millisecond-precise, so an older message just + // beyond the page can tie the oldest fetched timestamp. Preserve ties + // rather than guessing that an unseen boundary message was deleted. + sourceTimestamp > oldestFetchedTimestamp ); - newMessages++; - } else { - // We have this message - check if content changed (offline edit detection) - if (stored.content !== discordMsg.content) { - this.ctx.editMessage(stored.internalId, [ - { type: 'text', text: discordMsg.content }, - ]); - editedMessages++; + if (isInsideFetchedWindow && !discordIds.has(externalId) && !live.has(externalId)) { + // Message exists in our store but not in Discord - it was deleted + // Note: This only catches deletes within the scrollback window. + // A message the live path wrote while the sync ran is newer than + // the fetch, so it stays. One shard's id takes out a body whole. + ctx.removeMessage(stored.id); + deletedMessages++; } } - } - // Check for deletes only inside the history we actually fetched. A short - // response covers complete available history; a full page has an older - // boundary whose tied millisecond must be preserved. - const fetchedCompleteHistory = discordMessages.length < scrollback; - const oldestFetchedTimestamp = discordMessages.length > 0 - ? Math.min(...discordMessages.map((msg) => msg.timestamp.getTime())) - : undefined; - for (const [externalId, stored] of storedMessageMap) { - const isInsideFetchedWindow = fetchedCompleteHistory || ( - stored.sourceTimestamp !== undefined && - oldestFetchedTimestamp !== undefined && - // Discord timestamps are millisecond-precise, so an older message just - // beyond the page can tie the oldest fetched timestamp. Preserve ties - // rather than guessing that an unseen boundary message was deleted. - stored.sourceTimestamp > oldestFetchedTimestamp - ); - if (isInsideFetchedWindow && !discordMessageMap.has(externalId)) { - // Message exists in our store but not in Discord - it was deleted - // Note: This only catches deletes within the scrollback window - this.ctx.removeMessage(stored.internalId); - deletedMessages++; + // Update last read + if (discordMessages.length > 0) { + // Messages are typically newest first + const newestId = discordMessages[0].id; + this.state.lastReadMessageId[channelId] = newestId; } + + return { newMessages, editedMessages, deletedMessages }; + } finally { + this.liveDuringSync.delete(live); } + } - // Update last read - if (discordMessages.length > 0) { - // Messages are typically newest first - const newestId = discordMessages[0].id; - this.state.lastReadMessageId[channelId] = newestId; + /** A channel's stored Discord rows by their Discord message id, whole: an + * edit replaces a row's own text and keeps what the host added beside it. */ + private storedByDiscordId(ctx: ModuleContext, channelId: string): Map { + const rows = new Map(); + for (const msg of ctx.queryMessages({ source: 'discord', metadata: { channelId } }).messages) { + const external = msg.metadata?.external as { id?: string } | undefined; + if (external?.id) rows.set(external.id, msg); } + return rows; + } - return { newMessages, editedMessages, deletedMessages }; + /** The stored row of a Discord message, by its id in the row's metadata. + * The module context's findMessageByExternalId holds only rows the module + * stored through its context, not those a process response added, so it + * misses what the live path stored. */ + private storedRow(discordMessageId: string): StoredMessage | undefined { + return this.ctx?.queryMessages({ source: 'discord', externalIds: [discordMessageId] }).messages[0]; + } + + /** Marks a Discord message as the live path's for every history sync running now. */ + private noteLive(discordMessageId: string): void { + for (const live of this.liveDuringSync) live.add(discordMessageId); } private async handleDiscordMessage(message: DiscordMessageData): Promise { @@ -914,19 +1099,18 @@ export class DiscordModule implements Module { } /** - * Convert Discord message to ContentBlock array. - * Includes text content and any supported attachments (images, text files). + * Convert a Discord message, live or from history, to ContentBlock array: + * its own text first, '' when it has none, then any supported attachments + * (images, text files) and notes for the rest, as TEXT_FIRST marks. */ - private async convertMessageToContent(message: DiscordMessageData): Promise { + private async convertMessageToContent( + message: { id: string; content: string; attachments?: readonly DiscordAttachment[] }, + ): Promise { + // What the host adds after the message's own text: one block per attachment. const blocks: ContentBlock[] = []; - // Add text content if present - if (message.content.trim()) { - blocks.push({ type: 'text', text: message.content }); - } - // Process attachments - for (const attachment of message.attachments) { + for (const attachment of message.attachments ?? []) { const contentType = attachment.contentType?.toLowerCase() ?? ''; const filename = attachment.filename.toLowerCase(); @@ -973,12 +1157,7 @@ export class DiscordModule implements Module { }); } - // Ensure we have at least empty text if nothing else - if (blocks.length === 0) { - blocks.push({ type: 'text', text: '[Empty message]' }); - } - - return blocks; + return textFirst(ownText(message.content), blocks); } /** @@ -1056,9 +1235,13 @@ export class DiscordModule implements Module { createThread: input.createThread, }); - // Track for thought-editing + // Track for thought-editing: the message is out, whether or not its + // thread was started. this.lastSentMessage = { channelId: input.channelId, messageId: result.messageId }; + if (result.threadFailure !== undefined) { + return { success: false, error: result.threadFailure, isError: true }; + } return { success: true, data: { messageId: result.messageId } }; } @@ -1222,6 +1405,11 @@ export class DiscordModule implements Module { authorId: m.authorId, isBot: m.isBot, content: m.content, + // Not their URLs: a link Discord signs works for anyone holding it + // until it expires, and an agent could repeat it where others read. + ...(m.attachments?.length + ? { attachments: m.attachments.map(({ filename, contentType, size }) => ({ filename, contentType, size })) } + : {}), timestamp: formatZonedDateTime(m.timestamp, this.timeZone), replyTo: m.replyTo, })), diff --git a/src/modules/discord/types.ts b/src/modules/discord/types.ts index 8b663d66c..5168f4a36 100644 --- a/src/modules/discord/types.ts +++ b/src/modules/discord/types.ts @@ -264,6 +264,13 @@ export interface HistoryMessage { /** Message content */ content: string; + /** Attachments, as the live path's DiscordMessageData carries them. A + * client that doesn't map them leaves this out: the history sync then + * stores a message it missed without them, and rebuilds no row, since it + * can't tell what the row should hold, though it still changes a row's + * text as a live edit does. */ + attachments?: DiscordAttachment[]; + /** Timestamp */ timestamp: Date; @@ -403,7 +410,13 @@ export interface DiscordClientInterface { /** Get the bot's user ID (available after connect) */ getBotUserId(): string | null; - /** Send a message to a channel */ + /** + * Send a message to a channel, and start the thread `createThread` asks + * for when the channel can hold threads; in one that can't, such as a DM, + * no thread is started and nothing says so. When this resolves, the + * message was sent. `threadFailure`, when there is one, is why the thread + * it tried to start wasn't. + */ sendMessage( channelId: string, content: string, @@ -411,7 +424,7 @@ export interface DiscordClientInterface { replyTo?: string; createThread?: { name: string }; } - ): Promise<{ messageId: string }>; + ): Promise<{ messageId: string; threadFailure?: string }>; /** Send a DM to a user */ sendDM(userId: string, content: string): Promise<{ messageId: string }>; diff --git a/src/types/module.ts b/src/types/module.ts index c3b8b45ef..4f86bf997 100644 --- a/src/types/module.ts +++ b/src/types/module.ts @@ -15,6 +15,8 @@ import type { TraceEventListener } from './trace.js'; export interface MessageQuery { /** Filter by external source (e.g., 'discord') */ source?: string; + /** Filter by external IDs: messages whose metadata.external.id is one of these. An empty list filters nothing. */ + externalIds?: string[]; /** Filter by participant name */ participant?: string; /** Filter by metadata fields (shallow match) */ diff --git a/test/discord-attachment-notice.test.ts b/test/discord-attachment-notice.test.ts index 637be3297..f3a1fe92c 100644 --- a/test/discord-attachment-notice.test.ts +++ b/test/discord-attachment-notice.test.ts @@ -15,7 +15,8 @@ interface ConversionHarness { convertMessageToContent(message: DiscordMessageData): Promise; } -/** A message whose only content is one small text attachment. */ +/** A message whose only content is one small text attachment, so its own + * text, stored first, is ''. */ function messageWithTextFile(filename: string): DiscordMessageData { return { id: 'm1', channelId: 'c1', guildId: 'g1', authorId: 'u1', authorName: 'User', isBot: false, @@ -41,7 +42,7 @@ test('a text attachment whose fetch is answered with a status that isn\'t OK rea const warn = t.mock.method(console, 'warn', () => {}); let response: Response | undefined; const blocks = await convert(t, async () => (response = new Response('gone', { status: 404 }))); - assert.deepEqual(blocks, [{ type: 'text', text: '[Attachment: notes.txt (failed to fetch)]' }]); + assert.deepEqual(blocks, [{ type: 'text', text: '' }, { type: 'text', text: '[Attachment: notes.txt (failed to fetch)]' }]); // The operator's line names the file, the message and the status, and the // unread answer is cancelled, so its connection is released. assert.deepEqual(warn.mock.calls.map((c) => c.arguments.join(' ')), ['Discord: Attachment notes.txt in message m1 wasn\'t fetched: HTTP 404']); @@ -58,7 +59,7 @@ test('a filename that could break a log line is printed quoted, as one field', a test('a text attachment whose fetch throws reads the same', async (t) => { const warn = t.mock.method(console, 'warn', () => {}); const blocks = await convert(t, async () => { throw new TypeError('fetch failed'); }); - assert.deepEqual(blocks, [{ type: 'text', text: '[Attachment: notes.txt (failed to fetch)]' }]); + assert.deepEqual(blocks, [{ type: 'text', text: '' }, { type: 'text', text: '[Attachment: notes.txt (failed to fetch)]' }]); assert.equal(warn.mock.calls.length, 1); assert.match(String(warn.mock.calls[0].arguments[0]), /^Discord: Attachment notes\.txt in message m1 wasn't fetched:$/); assert.equal((warn.mock.calls[0].arguments[1] as Error).message, 'fetch failed'); @@ -66,5 +67,5 @@ test('a text attachment whose fetch throws reads the same', async (t) => { test('a text attachment that is fetched is read whole', async (t) => { const blocks = await convert(t, async () => new Response('line one', { status: 200 })); - assert.deepEqual(blocks, [{ type: 'text', text: '\n--- File: notes.txt ---\nline one\n--- End of notes.txt ---\n' }]); + assert.deepEqual(blocks, [{ type: 'text', text: '' }, { type: 'text', text: '\n--- File: notes.txt ---\nline one\n--- End of notes.txt ---\n' }]); }); diff --git a/test/discord-history-host-blocks.test.ts b/test/discord-history-host-blocks.test.ts new file mode 100644 index 000000000..0a9a312ec --- /dev/null +++ b/test/discord-history-host-blocks.test.ts @@ -0,0 +1,601 @@ +/** + * A stored Discord message keeps what the host added beside its own text: + * images, inlined file text and attachment notes. The live path stores the + * message's own text first, '' when it has none, and marks the row so; an + * edit or a history sync then replaces that block alone. A row stored before + * the mark is left alone by a live edit unless its text really changed. + * History carries attachments, so a message the sync missed is stored as the + * live path would have stored it, a row that doesn't hold what the live path + * would store is stored through it again, and fetch_history lists them. The + * sync leaves what the live path wrote while it ran. + * + * Run: node --import tsx --test test/discord-history-host-blocks.test.ts + */ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { Collection } from 'discord.js'; +import { DiscordModule, DiscordJsClient } from '../src/modules/discord/index.js'; +import type { + DiscordAttachment, + DiscordClientInterface, + DiscordMessageData, + HistoryMessage, +} from '../src/modules/discord/types.js'; +import type { ModuleContext, ProcessEvent, ProcessState } from '../src/types/index.js'; + +type Block = { type: string; text?: string; source?: unknown }; +interface Row { + id: string; + participant: string; + content: Block[]; + metadata: Record & { external: { source: string; id: string }; channelId: string }; + timestamp: Date; + bodyGroupId?: string; +} + +interface Harness { + ctx: ModuleContext | null; + handleDiscordMessage(message: DiscordMessageData): Promise; + syncChannelHistory(channelId: string): Promise<{ newMessages: number; editedMessages: number; deletedMessages: number }>; +} + +const image: DiscordAttachment = { id: 'a1', filename: 'cat.png', url: 'https://cdn.example/cat.png', contentType: 'image/png', size: 2_000 }; +const notes: DiscordAttachment = { id: 'a2', filename: 'notes.txt', url: 'https://cdn.example/notes.txt', contentType: 'text/plain', size: 9 }; +const imageBlock = { type: 'image', source: { type: 'url', url: image.url } }; +const fileBlock = { type: 'text', text: '\n--- File: notes.txt ---\nfile body\n--- End of notes.txt ---\n' }; +const text = (t: string) => ({ type: 'text', text: t }); + +function row(id: string, content: Block[], textFirst: boolean, timestamp = 1_000): Row { + return { + id: `stored-${id}`, + participant: 'User', + content, + metadata: { + external: { source: 'discord', id }, + channelId: 'c1', + timestamp, + ...(textFirst ? { discordTextFirst: true } : {}), + }, + timestamp: new Date(timestamp), + }; +} + +function setup(rows: Row[], client: Partial = {}) { + const module = new DiscordModule({ sendTyping: async () => {}, ...client } as unknown as DiscordClientInterface, { token: 't', autoTyping: false }); + const harness = module as unknown as Harness; + const events: ProcessEvent[] = []; + const added: Array<{ content: Block[]; metadata: Record }> = []; + const edits: Array<{ id: string; content: Block[] }> = []; + const removed: string[] = []; + // As context-manager's store does, an edit or removal of a row it doesn't + // hold throws, and so does an edit of a shard of a body. + const held = (id: string) => { + const r = rows.find((candidate) => candidate.id === id); + if (!r) throw new Error(`Message not found: ${id}`); + return r; + }; + const edit = (id: string, content: Block[]) => { + const r = held(id); + if (r.bodyGroupId) throw new Error(`Cannot edit shard ${id}`); + r.content = content; + }; + // As the framework does, removing a shard takes out its whole body. + const remove = (id: string) => { + const r = held(id); + for (const gone of r.bodyGroupId ? rows.filter((other) => other.bodyGroupId === r.bodyGroupId) : [r]) rows.splice(rows.indexOf(gone), 1); + }; + harness.ctx = { + queryMessages: (q: { source?: string; externalIds?: string[]; metadata?: Record }) => { + const messages = rows.filter((r) => (q.source === undefined || r.metadata.external.source === q.source) + && (!q.externalIds || q.externalIds.includes(r.metadata.external.id)) + && (q.metadata?.channelId === undefined || r.metadata.channelId === q.metadata.channelId)); + return { messages, totalCount: messages.length }; + }, + getAgents: () => [{ name: 'Sol' }], + addMessage: (_participant: string, content: Block[], metadata: Record) => { + added.push({ content, metadata }); + return `stored-new-${added.length}`; + }, + editMessage: (id: string, content: Block[]) => { edit(id, content); edits.push({ id, content }); }, + removeMessage: (id: string) => { remove(id); removed.push(id); }, + pushEvent: (event: ProcessEvent) => { events.push(event); }, + setState: () => {}, + } as unknown as ModuleContext; + // As the framework's is, the context's external-id index holds only rows a + // module stored through the context, so it misses every row in these tests. + const state = { + getState: () => null, + findMessageByExternalId: () => null, + } as unknown as ProcessState; + // The framework's part: a live event's response, written to the store. + const apply = async (event: Record) => { + const response = await module.onProcess(event as unknown as ProcessEvent, state); + for (const message of response.addMessages ?? []) { + const metadata = message.metadata as Row['metadata']; + rows.push({ id: `stored-${metadata.external.id}`, participant: message.participant, content: message.content as Block[], metadata, timestamp: new Date(metadata.timestamp as number) }); + } + for (const change of response.editMessages ?? []) edit(change.messageId, change.content as Block[]); + for (const id of response.removeMessages ?? []) remove(id); + return response; + }; + return { module, harness, events, added, edits, removed, state, apply, rows }; +} + +function live(id: string, content: string, attachments: DiscordAttachment[] = []): DiscordMessageData { + return { + id, channelId: 'c1', guildId: 'g1', authorId: 'u1', authorName: 'User', isBot: false, content, + isReply: false, mentionedUserIds: [], attachments, timestamp: new Date(1_000), + }; +} + +function history(id: string, content: string, timestamp: number, attachments?: DiscordAttachment[]): HistoryMessage { + return { id, authorId: 'u1', authorName: 'User', isBot: false, content, timestamp: new Date(timestamp), ...(attachments ? { attachments } : {}) }; +} + +test('a live message is stored with its own text first, \'\' when it has none, then what the host added, and is marked so', async () => { + const { module, harness, events, state } = setup([]); + await harness.handleDiscordMessage(live('m1', '', [image])); + await harness.handleDiscordMessage(live('m2', 'hello')); + await harness.handleDiscordMessage(live('m3', ' ')); + const stored = []; + for (const event of events) { + const response = await module.onProcess(event, state); + const [message] = response.addMessages ?? []; + stored.push({ content: message?.content, textFirst: (message?.metadata as Record)?.discordTextFirst }); + } + assert.deepEqual(stored, [ + { content: [text(''), imageBlock], textFirst: true }, + { content: [text('hello')], textFirst: true }, + { content: [text(''), text('[Empty message]')], textFirst: true }, + ]); +}); + +test('an edit replaces a marked row\'s own text and keeps every block the host added', async () => { + const slack = row('m1', [text('another source')], true); + const { module, state } = setup([ + // Another source's message with the same id, which no Discord edit reaches. + { ...slack, id: 'slack-m1', metadata: { ...slack.metadata, external: { source: 'slack', id: 'm1' } } }, + row('m1', [text('old'), fileBlock, imageBlock], true), + row('m2', [text('only text')], true), + row('m3', [text(''), text('[Empty message]')], true), + row('m4', [text(''), imageBlock], true), + row('m5', [text('x'), imageBlock], true), + ]); + const edit = (id: string, newContent: string) => + module.onProcess({ type: 'discord:edit', discordMessageId: id, newContent } as unknown as ProcessEvent, state); + assert.deepEqual(await edit('m1', 'new'), { editMessages: [{ messageId: 'stored-m1', content: [text('new'), fileBlock, imageBlock] }] }); + // An update that leaves the text as it was, such as a link's embed + // arriving, changes nothing. + assert.deepEqual(await edit('m1', 'old'), {}); + // An edit that leaves no text: the marker comes back only when nothing + // else is left, and goes once there is text again. + assert.deepEqual(await edit('m2', ''), { editMessages: [{ messageId: 'stored-m2', content: [text(''), text('[Empty message]')] }] }); + assert.deepEqual(await edit('m4', ''), {}); + assert.deepEqual(await edit('m5', ''), { editMessages: [{ messageId: 'stored-m5', content: [text(''), imageBlock] }] }); + assert.deepEqual(await edit('m3', 'now'), { editMessages: [{ messageId: 'stored-m3', content: [text('now')] }] }); + // A message nobody stored is left alone. + assert.deepEqual(await edit('m9', 'x'), {}); +}); + +test('a live edit leaves a row stored before the mark alone unless its text is there and differs, and then puts the text in place of the first block, keeping the rest; a sync without attachments to compare rebuilds none, and changes a text as an edit does', async () => { + const pdfNote = text('[Attachment: x.pdf (application/pdf)]'); + const { module, harness, edits, state } = setup([ + row('l1', [text('own'), fileBlock], false), + row('l2', [pdfNote], false), + row('l3', [imageBlock], false), + row('l4', [pdfNote, imageBlock], false), + row('l5', [text('[Empty message]')], false), + row('l6', [text('own'), fileBlock], false), + // As the history sync rebuilds an empty message stored before the mark. + row('l7', [text(''), text('[Empty message]')], false), + row('l8', [fileBlock, imageBlock], false), + ], { + fetchHistory: async () => ['l8', 'l7', 'l5', 'l4', 'l3', 'l2'].map((id) => history(id, '', 1_000)) + .concat(history('l6', 'changed while away', 1_000), history('l1', 'own', 1_000)), + }); + const edit = (id: string, newContent: string) => + module.onProcess({ type: 'discord:edit', discordMessageId: id, newContent } as unknown as ProcessEvent, state); + assert.deepEqual(await edit('l1', 'own'), {}); + assert.deepEqual(await edit('l1', ''), {}, 'a blank edit leaves it: its first block may be what the host added'); + assert.deepEqual(await harness.syncChannelHistory('c1'), { newMessages: 0, editedMessages: 1, deletedMessages: 0 }); + // With no attachments to compare, the sync rebuilds nothing, so l6 keeps its file. + assert.deepEqual(edits, [{ id: 'stored-l6', content: [text('changed while away'), fileBlock] }]); + // A real change takes the first block's place when it's text, and keeps the rest. + assert.deepEqual(await edit('l1', 'changed'), { editMessages: [{ messageId: 'stored-l1', content: [text('changed'), fileBlock] }] }); + assert.deepEqual(await edit('l5', 'now'), { editMessages: [{ messageId: 'stored-l5', content: [text('now')] }] }); + // The marker goes once there's text. + assert.deepEqual(await edit('l7', 'now'), { editMessages: [{ messageId: 'stored-l7', content: [text('now')] }] }); + // A row that begins with what the host added, an image, gets the text before it. + assert.deepEqual(await edit('l3', 'caption'), { editMessages: [{ messageId: 'stored-l3', content: [text('caption'), imageBlock] }] }); + // The worst case: a message with no text of its own, stored beginning with + // a file's note or contents, loses that block, where the rewrite lost the + // image too. + assert.deepEqual(await edit('l4', 'added'), { editMessages: [{ messageId: 'stored-l4', content: [text('added'), imageBlock] }] }); + assert.deepEqual(await edit('l8', 'added'), { editMessages: [{ messageId: 'stored-l8', content: [text('added'), imageBlock] }] }); +}); + +test('a history sync replaces only a marked row\'s own text, and stores a missed message as the live path would', async (t) => { + t.mock.method(globalThis, 'fetch', async (url: string) => { + assert.equal(url, notes.url); + return new Response('file body', { status: 200 }); + }); + const note = text('[Attachment: x.pdf (application/pdf)]'); + const { harness, added, edits } = setup([ + row('s1', [text('same'), note], true, 1_000), + row('s2', [text('before'), imageBlock], true, 2_000), + ], { + fetchHistory: async () => [ + history('n2', '', 5_000, [image]), + history('n1', 'with a file', 4_000, [notes]), + history('s2', 'after', 2_000, [image]), + history('s1', 'same', 1_000), + ], + }); + assert.deepEqual(await harness.syncChannelHistory('c1'), { newMessages: 2, editedMessages: 1, deletedMessages: 0 }); + assert.deepEqual(edits, [{ id: 'stored-s2', content: [text('after'), imageBlock] }]); + assert.deepEqual(added.map((a) => [a.content, a.metadata.discordTextFirst]), [ + [[text('with a file'), fileBlock], true], + [[text(''), imageBlock], true], + ]); + // Stored with what a live edit or delete, and the next sync, find a row by. + assert.deepEqual(added.map((a) => [a.metadata.external, a.metadata.channelId]), [ + [{ source: 'discord', id: 'n1' }, 'c1'], + [{ source: 'discord', id: 'n2' }, 'c1'], + ]); +}); + +test('a history sync stores nothing the live path stored while a missed message\'s attachments were fetched, nor anything once the module stopped or started again', async (t) => { + const rows: Row[] = []; + let during = () => {}; + t.mock.method(globalThis, 'fetch', async () => { + during(); + return new Response('file body', { status: 200 }); + }); + const { harness, added } = setup(rows, { fetchHistory: async () => [history('n1', 'with a file', 1_000, [notes])] }); + const original = harness.ctx; + during = () => { rows.push(row('n1', [text('with a file'), fileBlock], true)); }; + assert.deepEqual(await harness.syncChannelHistory('c1'), { newMessages: 0, editedMessages: 0, deletedMessages: 0 }); + assert.deepEqual(added, []); + rows.length = 0; + during = () => { harness.ctx = null; }; + assert.deepEqual(await harness.syncChannelHistory('c1'), { newMessages: 0, editedMessages: 0, deletedMessages: 0 }); + assert.deepEqual(added, []); + // Nor into the context of a module started again meanwhile. + const next = setup([]); + harness.ctx = original; + during = () => { harness.ctx = next.harness.ctx; }; + assert.deepEqual(await harness.syncChannelHistory('c1'), { newMessages: 0, editedMessages: 0, deletedMessages: 0 }); + assert.deepEqual([added, next.added], [[], []]); +}); + +test('a message the live path stored is neither rebuilt nor fetched again by a history sync', async (t) => { + const pdf: DiscordAttachment = { id: 'a3', filename: 'x.pdf', url: 'https://cdn.example/x.pdf', contentType: 'application/pdf', size: 50 }; + const lost: DiscordAttachment = { id: 'a4', filename: 'gone.txt', url: 'https://cdn.example/gone.txt', contentType: 'text/plain', size: 9 }; + const fetched: string[] = []; + t.mock.method(globalThis, 'fetch', async (url: string) => { + fetched.push(url); + return url === lost.url ? new Response('', { status: 404 }) : new Response('file body', { status: 200 }); + }); + // An image, a file's text, a file not fetched (its notice) and a note; a + // file alone; text alone; and nothing at all (the empty marker). + const messages = [live('m1', 'all of them', [image, notes, lost, pdf]), live('m2', '', [notes]), live('m3', 'plain'), live('m4', '')]; + const rows: Row[] = []; + const { harness, events, added, edits, apply } = setup(rows, { + fetchHistory: async () => [...messages].reverse().map((m) => history(m.id, m.content, 1_000, m.attachments)), + }); + for (const message of messages) await harness.handleDiscordMessage(message); + for (const event of events) await apply(event as unknown as Record); + assert.deepEqual(rows.map((r) => r.content), [ + [text('all of them'), imageBlock, fileBlock, text('[Attachment: gone.txt (failed to fetch)]'), text('[Attachment: x.pdf (application/pdf)]')], + [text(''), fileBlock], + [text('plain')], + [text(''), text('[Empty message]')], + ]); + fetched.length = 0; + assert.deepEqual(await harness.syncChannelHistory('c1'), { newMessages: 0, editedMessages: 0, deletedMessages: 0 }); + assert.deepEqual([edits, added, fetched], [[], [], []]); +}); + +test('a history sync rebuilds a row that doesn\'t hold what the live path would store, blanked or stripped before the mark, still holding a removed attachment or text its message lost, or a caption its message lost, and a row whose text changed keeps its file', async (t) => { + t.mock.method(globalThis, 'fetch', async () => new Response('file body', { status: 200 })); + const pdf: DiscordAttachment = { id: 'a3', filename: 'x.pdf', url: 'https://cdn.example/x.pdf', contentType: 'application/pdf', size: 50 }; + const pdfNote = text('[Attachment: x.pdf (application/pdf)]'); + const { module, state, harness, edits, added } = setup([ + row('b1', [text('')], false, 1_000), + row('b2', [text('own')], false, 2_000), + row('b3', [text('old'), fileBlock], false, 3_000), + row('b4', [pdfNote], false, 4_000), + row('b5', [text('keep'), imageBlock], true, 5_000), + row('b6', [imageBlock], false, 6_000), + row('b7', [text('was here'), fileBlock], false, 7_000), + row('b8', [text('')], false, 8_000), + // An earlier sync stripped b9 to its caption, and the caption went while + // its file stayed: one block where one is expected. + row('b9', [text('old caption')], false, 9_000), + // b10's own text reads as the marker. + row('b10', [text('[Empty message]'), fileBlock], false, 10_000), + ], { + fetchHistory: async () => [ + history('b10', '[Empty message]', 10_000, [notes]), + history('b9', '', 9_000, [notes]), + history('b8', '', 8_000, []), + history('b7', '', 7_000, [notes]), + history('b6', '', 6_000, []), + history('b5', 'keep', 5_000, []), + history('b4', '', 4_000, [pdf]), + history('b3', 'new', 3_000, [notes]), + history('b2', 'own', 2_000, [pdf]), + history('b1', '', 1_000, [notes]), + ], + }); + assert.deepEqual(await harness.syncChannelHistory('c1'), { newMessages: 0, editedMessages: 9, deletedMessages: 0 }); + assert.deepEqual(edits, [ + // An attachment-only message an earlier sync blanked. + { id: 'stored-b1', content: [text(''), fileBlock] }, + // One an earlier sync stripped of its note. + { id: 'stored-b2', content: [text('own'), pdfNote] }, + // One whose text changed while the client was away keeps its file. + { id: 'stored-b3', content: [text('new'), fileBlock] }, + // b4 holds what the live path stored before the mark, but its message has + // no text, so its first block can't be told from a caption it lost, and + // it's stored again, once. + { id: 'stored-b4', content: [text(''), pdfNote] }, + // A marked row whose image was removed from the message loses it. + { id: 'stored-b5', content: [text('keep')] }, + // So does an image-only message stored before the mark. + { id: 'stored-b6', content: [text(''), text('[Empty message]')] }, + // One whose message lost its text and kept its file, which a live edit + // left alone, since a blank text there may be the host's first block. + { id: 'stored-b7', content: [text(''), fileBlock] }, + // A message with neither text nor attachments, which an earlier sync blanked. + { id: 'stored-b8', content: [text(''), text('[Empty message]')] }, + // The caption its message lost goes, and its file comes back. + { id: 'stored-b9', content: [text(''), fileBlock] }, + // b10's text and file are both there, so it's left alone. + ]); + assert.deepEqual(added, []); + // Each row it rebuilt holds what the next sync expects, so that sync + // writes nothing. + assert.deepEqual(await harness.syncChannelHistory('c1'), { newMessages: 0, editedMessages: 0, deletedMessages: 0 }); + // The sync can't mark what it rebuilt, since an edit keeps metadata, and a + // live edit of it keeps the note the sync gave back. + assert.deepEqual(await module.onProcess({ type: 'discord:edit', discordMessageId: 'b2', newContent: 'newer' } as unknown as ProcessEvent, state), + { editMessages: [{ messageId: 'stored-b2', content: [text('newer'), pdfNote] }] }); +}); + +test('a rebuild whose file wasn\'t fetched writes nothing over a row holding text besides its message\'s own, which a later sync rebuilds once the file is fetched, and gives a row missing its file the notice', async (t) => { + let reachable = false; + t.mock.method(globalThis, 'fetch', async () => reachable ? new Response('file body', { status: 200 }) : new Response('', { status: 503 })); + t.mock.method(console, 'warn', () => {}); + const more = text('\n--- File: more.txt ---\nmore\n--- End of more.txt ---\n'); + const notFetched = text('[Attachment: notes.txt (failed to fetch)]'); + const rows: Row[] = [ + // A message without text whose row begins with its file's text. + row('p1', [fileBlock], false, 1_000), + // A file removed while away from a row holding both files' texts. + row('p2', [text('both'), fileBlock, more], false, 2_000), + // Rows an earlier sync stripped of their file, to blank text and to the + // caption: a notice costs them nothing. + row('p3', [text('')], false, 3_000), + row('p4', [text('caption')], false, 4_000), + // A message whose own text reads as the notice for its file. + row('p5', [notFetched, fileBlock, more], true, 5_000), + // An attachment-only message that has gained text since its row was stored. + row('p6', [fileBlock], false, 6_000), + ]; + const { harness } = setup(rows, { + fetchHistory: async () => [ + history('p6', 'gained', 6_000, [notes]), + history('p5', notFetched.text, 5_000, [notes]), + history('p4', 'caption', 4_000, [notes]), + history('p3', '', 3_000, [notes]), + history('p2', 'both', 2_000, [notes]), + history('p1', '', 1_000, [notes]), + ], + }); + const held = () => rows.map((r) => r.content); + assert.deepEqual(await harness.syncChannelHistory('c1'), { newMessages: 0, editedMessages: 2, deletedMessages: 0 }); + assert.deepEqual(held(), [[fileBlock], [text('both'), fileBlock, more], [text(''), notFetched], [text('caption'), notFetched], [notFetched, fileBlock, more], [fileBlock]]); + reachable = true; + assert.deepEqual(await harness.syncChannelHistory('c1'), { newMessages: 0, editedMessages: 4, deletedMessages: 0 }); + assert.deepEqual(held(), [[text(''), fileBlock], [text('both'), fileBlock], [text(''), notFetched], [text('caption'), notFetched], [notFetched, fileBlock], [text('gained'), fileBlock]]); + assert.deepEqual(await harness.syncChannelHistory('c1'), { newMessages: 0, editedMessages: 0, deletedMessages: 0 }); +}); + +test('a history sync leaves what the live path wrote while it fetched, stores a missed message as it fetched it though a live edit came for it, and writes no row that has gone', async (t) => { + const pdf: DiscordAttachment = { id: 'a3', filename: 'x.pdf', url: 'https://cdn.example/x.pdf', contentType: 'application/pdf', size: 50 }; + const rows: Row[] = [ + row('e1', [text('before')], true, 1_000), + row('e2', [text('before')], true, 2_000), + row('e3', [text('stays')], true, 3_000), + row('e4', [text('before')], true, 4_000), + // A row the sync would store again through the live conversion: one + // that lacks its file, and one stored before the mark that lacks its note. + row('e5', [text('before')], true, 4_500), + row('e6', [text('own')], false, 4_800), + ]; + let during = async () => {}; + t.mock.method(globalThis, 'fetch', async () => { + const now = during; + during = async () => {}; + await now(); + return new Response('file body', { status: 200 }); + }); + const { module, harness, edits, added, removed, state, apply } = setup(rows, { + // While the sync fetches history, another writer removes e4. + fetchHistory: async () => { + rows.splice(rows.findIndex((r) => r.id === 'stored-e4'), 1); + return [ + history('n3', 'sent while away', 7_000, []), + history('n2', 'missed, then deleted', 6_000, [notes]), + history('n1', 'missed', 5_000, [notes]), + history('e6', 'own', 4_800, [pdf]), + history('e5', 'before', 4_500, [notes]), + history('e4', 'edited while away', 4_000, []), + history('e3', 'stays', 3_000, []), + history('e2', 'edited while away', 2_000, []), + history('e1', 'edited while away', 1_000, []), + ]; + }, + }); + // While the sync fetches e5's file, the first it fetches: e1 is edited + // live and e2 deleted; e5 is removed by another writer; e6 gets an update + // that leaves its text as it was; the first missed message is edited and + // the second deleted; and the third arrives live, its add written only + // once the sync is done, as the framework may write it after other + // modules have answered. + let arrival: Awaited> | undefined; + during = async () => { + await apply({ type: 'discord:edit', discordMessageId: 'e1', newContent: 'edited live' }); + await apply({ type: 'discord:delete', discordMessageId: 'e2' }); + rows.splice(rows.findIndex((r) => r.id === 'stored-e5'), 1); + assert.deepEqual(await apply({ type: 'discord:edit', discordMessageId: 'e6', newContent: 'own' }), {}); + assert.deepEqual(await apply({ type: 'discord:edit', discordMessageId: 'n1', newContent: 'missed, then edited' }), {}); + await apply({ type: 'discord:delete', discordMessageId: 'n2' }); + arrival = await module.onProcess({ + type: 'discord:message', discordMessageId: 'n3', channelId: 'c1', guildId: 'g1', authorId: 'u1', authorName: 'User', + content: 'sent while away', contentBlocks: [text('sent while away')], timestamp: 7_000, attachments: [], + } as unknown as ProcessEvent, state); + }; + assert.deepEqual(await harness.syncChannelHistory('c1'), { newMessages: 1, editedMessages: 1, deletedMessages: 0 }); + // e6's update wrote nothing, so the sync still stores it again with its note. + const pdfNote = text('[Attachment: x.pdf (application/pdf)]'); + assert.deepEqual(rows.map((r) => [r.id, r.content]), [['stored-e1', [text('edited live')]], ['stored-e3', [text('stays')]], ['stored-e6', [text('own'), pdfNote]]]); + assert.deepEqual([edits.map((e) => e.id), removed], [['stored-e6'], []]); + // n1 with the text the sync fetched: the live edit that came before any + // row held it was dropped. e4 and e5, removed meanwhile, aren't brought + // back. + assert.deepEqual(added.map((a) => a.content), [[text('missed'), fileBlock]]); + assert.deepEqual(arrival?.addMessages?.map((m) => m.content), [[text('sent while away')]]); +}); + +test('a history sync removes no message the live path stored or was told was deleted while it ran, which its fetch couldn\'t hold', async (t) => { + const rows: Row[] = [row('d1', [text('deleted on Discord before the fetch')], true, 1_000)]; + let during = async () => {}; + t.mock.method(globalThis, 'fetch', async () => { + const now = during; + during = async () => {}; + await now(); + return new Response('file body', { status: 200 }); + }); + const { module, harness, removed, state, apply } = setup(rows, { + fetchHistory: async () => [history('n1', 'missed', 2_000, [notes])], + }); + // While the sync fetches the missed message's file: a message posted after + // the fetch arrives live and is stored, and d1's delete arrives, its + // removal written only once the sync is done. + let deletion: Awaited> | undefined; + during = async () => { + await apply({ + type: 'discord:message', discordMessageId: 'n2', channelId: 'c1', guildId: 'g1', authorId: 'u1', authorName: 'User', + content: 'posted after the fetch', contentBlocks: [text('posted after the fetch')], timestamp: 3_000, attachments: [], + }); + deletion = await module.onProcess({ type: 'discord:delete', discordMessageId: 'd1' } as unknown as ProcessEvent, state); + }; + assert.deepEqual(await harness.syncChannelHistory('c1'), { newMessages: 1, editedMessages: 0, deletedMessages: 0 }); + assert.deepEqual([rows.map((r) => r.id), removed], [['stored-d1', 'stored-n2'], []]); + // d1 is still there for the live delete, which would otherwise find no row. + assert.deepEqual(deletion?.removeMessages, ['stored-d1']); +}); + +test('a history sync writes nothing once its module stopped while it fetched history', async () => { + const rows: Row[] = [row('s1', [text('old')], true, 1_000)]; + const s = setup(rows, { + fetchHistory: async () => { + s.harness.ctx = null; + return [history('s1', 'new', 1_000, [])]; + }, + }); + assert.deepEqual(await s.harness.syncChannelHistory('c1'), { newMessages: 0, editedMessages: 0, deletedMessages: 0 }); + assert.deepEqual([s.edits, s.added, s.removed], [[], [], []]); +}); + +test('a history sync leaves no record of itself once it ends, when it writes and when its fetch throws', async () => { + const running = (module: unknown) => (module as { liveDuringSync: Set }).liveDuringSync.size; + const wrote = setup([row('s1', [text('old')], true)], { fetchHistory: async () => [history('s1', 'new', 1_000, [])] }); + assert.deepEqual(await wrote.harness.syncChannelHistory('c1'), { newMessages: 0, editedMessages: 1, deletedMessages: 0 }); + assert.equal(running(wrote.module), 0); + const failed = setup([], { fetchHistory: async () => { throw new Error('gateway closed'); } }); + await assert.rejects(failed.harness.syncChannelHistory('c1'), /gateway closed/); + assert.equal(running(failed.module), 0); +}); + +test('a message stored as a body is removed whole by a live delete and the history sync, and its text is left as stored', async (t) => { + // A body: one Discord message stored as shards that share its metadata. + const body = (id: string, timestamp: number) => ['a', 'b'].map((part) => ({ + ...row(id, [text(`part ${part}`)], true, timestamp), id: `stored-${id}${part}`, bodyGroupId: `g-${id}`, + })); + const { harness, edits, removed, apply, rows } = setup([...body('s1', 1_000), ...body('s2', 2_000), ...body('s3', 3_000), ...body('s4', 4_000)], { + fetchHistory: async () => [history('s3', 'part a', 3_000, [notes]), history('s1', 'changed', 1_000, [])], + }); + const fetched: string[] = []; + t.mock.method(globalThis, 'fetch', async (url: string) => { fetched.push(url); return new Response('file body', { status: 200 }); }); + const warned: string[] = []; + t.mock.method(console, 'warn', (...args: unknown[]) => { warned.push(args.map(String).join(' ')); }); + // context-manager can't edit a body in place, so an edit leaves it as + // stored, and the host's log says so. + assert.deepEqual(await apply({ type: 'discord:edit', discordMessageId: 's1', newContent: 'changed' }), {}); + assert.deepEqual(warned, ['Discord: An update to message s1 wasn\'t applied: it\'s stored in shards, which can\'t be edited, so if its text changed, the agent still has the earlier text']); + assert.deepEqual(await apply({ type: 'discord:delete', discordMessageId: 's4' }), { removeMessages: ['stored-s4a'] }); + // As far as the sync can tell, s1's text changed, s2 was deleted and s3 + // gained a file. It removes s2 whole, and fetches nothing for a body. + assert.deepEqual(await harness.syncChannelHistory('c1'), { newMessages: 0, editedMessages: 0, deletedMessages: 1 }); + assert.deepEqual([edits, removed, fetched], [[], ['stored-s2b'], []]); + assert.deepEqual(rows.map((r) => r.id), ['stored-s1a', 'stored-s1b', 'stored-s3a', 'stored-s3b']); +}); + +test('a live delete removes the row the live path stored, which the context\'s external-id index doesn\'t hold', async () => { + const { module, state } = setup([row('m1', [text('hello')], true)]); + assert.deepEqual(await module.onProcess({ type: 'discord:delete', discordMessageId: 'm1' } as unknown as ProcessEvent, state), { removeMessages: ['stored-m1'] }); + assert.deepEqual(await module.onProcess({ type: 'discord:delete', discordMessageId: 'm9' } as unknown as ProcessEvent, state), {}); +}); + +test('a message event that carries no content blocks is stored from its text by the same rule', async () => { + const { module, state } = setup([]); + const stored = async (content: string) => { + const response = await module.onProcess({ + type: 'discord:message', discordMessageId: 'm1', channelId: 'c1', guildId: 'g1', authorId: 'u1', authorName: 'User', + content, timestamp: 1_000, attachments: [], + } as unknown as ProcessEvent, state); + return response.addMessages?.[0]?.content; + }; + assert.deepEqual(await stored('hello'), [text('hello')]); + assert.deepEqual(await stored(' '), [text(''), text('[Empty message]')]); + assert.deepEqual(await stored(''), [text(''), text('[Empty message]')]); +}); + +test('fetch_history lists each message\'s attachments, and leaves the list out of a message with none', async () => { + const { module } = setup([], { + fetchHistory: async () => [history('h2', 'look', 2_000, [image]), history('h1', 'plain', 1_000)], + }); + const result = await module.handleToolCall({ id: 'call-1', name: 'fetch_history', input: { channelId: 'c1' } }); + const messages = (result.data as { messages: Array> }).messages; + assert.deepEqual(messages[0]?.attachments, [{ filename: 'cat.png', contentType: 'image/png', size: 2_000 }]); + assert.equal('attachments' in (messages[1] ?? {}), false); +}); + +test('the client\'s history carries each message\'s attachments, as a live message does', async (t) => { + const client = new DiscordJsClient({ token: 't' }); + t.after(() => client.disconnect()); + const attachment = { id: 'a1', name: 'cat.png', url: image.url, contentType: 'image/png', size: 2_000 }; + const message = { + id: 'h1', author: { id: 'u1', username: 'User', bot: false }, content: 'look', createdAt: new Date(1_000), + reference: null, attachments: new Collection([['a1', attachment]]), + }; + const channels = (client as unknown as { client: { channels: { fetch: unknown } } }).client.channels; + channels.fetch = async () => ({ messages: { fetch: async () => new Collection([['h1', message]]) } }); + const [fetched] = await client.fetchHistory('c1'); + assert.deepEqual(fetched?.attachments, [image]); +}); + +test('the client passes on an edit that leaves no text, and not a partial message\'s missing text', async (t) => { + const client = new DiscordJsClient({ token: 't' }); + t.after(() => client.disconnect()); + const seen: Array<[string, string]> = []; + client.onMessageEdit((id, content) => { seen.push([id, content]); }); + const emitter = (client as unknown as { client: { emit(event: string, ...args: unknown[]): boolean } }).client; + emitter.emit('messageUpdate', {}, { id: 'm1', content: '' }); + emitter.emit('messageUpdate', {}, { id: 'm2', content: null }); + emitter.emit('messageUpdate', {}, { id: 'm3', content: 'edited' }); + assert.deepEqual(seen, [['m1', ''], ['m3', 'edited']]); +}); diff --git a/test/discord-send-thread.test.ts b/test/discord-send-thread.test.ts new file mode 100644 index 000000000..e883f68ed --- /dev/null +++ b/test/discord-send-thread.test.ts @@ -0,0 +1,67 @@ +/** + * A send whose message went out but whose thread wasn't started: the result + * stays the error it was, and the message counts as the one the bot last + * sent, so the agent's thoughts are edited into it. + * + * Run: node --import tsx --test test/discord-send-thread.test.ts + */ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { DiscordJsClient, DiscordModule } from '../src/modules/discord/index.js'; +import type { DiscordClientInterface } from '../src/modules/discord/types.js'; +import type { ToolResult } from '../src/types/index.js'; + +/** A DiscordJsClient whose channel lookup gives `channel`. Nothing connects. */ +function client(t: { after: (fn: () => Promise | void) => void }, channel: unknown): DiscordJsClient { + const djs = new DiscordJsClient({ token: 'test-token' }); + (djs as unknown as { client: { channels: { fetch: unknown } } }).client.channels.fetch = async () => channel; + t.after(() => djs.disconnect()); + return djs; +} + +/** A server text channel, which holds threads, whose sent message behaves as `message` does. */ +const textChannel = (message: Record) => ({ type: 0, send: async () => message, threads: {} }); + +test('a thread that isn\'t started after its message was sent resolves the send, with why', async (t) => { + const refused = client(t, textChannel({ id: 'M9', startThread: async () => { throw new Error('Missing Permissions'); } })); + assert.deepEqual(await refused.sendMessage('C1', 'hi', { createThread: { name: 'topic' } }), + { messageId: 'M9', threadFailure: 'Missing Permissions' }); + + const started = client(t, textChannel({ id: 'M6', startThread: async () => ({ id: 'M6' }) })); + assert.deepEqual(await started.sendMessage('C4', 'hi', { createThread: { name: 'topic' } }), { messageId: 'M6' }); + + // A channel without threads, such as a DM, is sent to without one, as before. + const dm = client(t, { type: 1, send: async () => ({ id: 'M8', startThread: async () => { throw new Error('not reached'); } }) }); + assert.deepEqual(await dm.sendMessage('D2', 'hi', { createThread: { name: 'topic' } }), { messageId: 'M8' }); +}); + +test('a send whose thread wasn\'t started stays an error, and the agent\'s thoughts are edited into its message, not the one before', async () => { + const edited: Array<[string, string, string]> = []; + let sent: { messageId: string; threadFailure?: string } = { messageId: 'M1' }; + const m = new DiscordModule({ + sendMessage: async () => sent, + fetchMessage: async () => 'hello', + editMessage: async (channelId: string, messageId: string, content: string) => { edited.push([channelId, messageId, content]); }, + } as unknown as DiscordClientInterface, { token: 'test-token' }); + const send = (input: Record): Promise => m.handleToolCall({ id: 'call-1', name: 'send', input }); + + assert.equal((await send({ channelId: 'C1', content: 'first' })).success, true); + sent = { messageId: 'M3', threadFailure: 'Missing Permissions' }; + assert.deepEqual(await send({ channelId: 'C1', content: 'hello', createThread: { name: 'topic' } }), + { success: false, error: 'Missing Permissions', isError: true }); + await m.onAgentSpeech('agent', [], { thoughts: [{ type: 'text', text: 'why' }] } as never); + assert.deepEqual(edited, [['C1', 'M3', '||why||\nhello']]); +}); + +test('a send whose client throws is the send\'s error, and records no message for the agent\'s thoughts', async () => { + const edited: unknown[] = []; + const m = new DiscordModule({ + sendMessage: async () => { throw new Error('Missing Permissions'); }, + fetchMessage: async () => 'hello', + editMessage: async (...args: unknown[]) => { edited.push(args); }, + } as unknown as DiscordClientInterface, { token: 'test-token' }); + assert.deepEqual(await m.handleToolCall({ id: 'call-1', name: 'send', input: { channelId: 'C1', content: 'hi' } }), + { success: false, error: 'Missing Permissions', isError: true }); + await m.onAgentSpeech('agent', [], { thoughts: [{ type: 'text', text: 'why' }] } as never); + assert.deepEqual(edited, []); +});