Fix: GIF animations not playing in Telegram#20
Fix: GIF animations not playing in Telegram#200xRabbidfly wants to merge 2 commits intoelizaos-plugins:1.xfrom
Conversation
- Separate GIF detection from generic image handling - Use ctx.replyWithAnimation() for GIFs instead of ctx.replyWithPhoto() - Prevents GIFs from being converted to static images - Add proper fallback handling for failed GIF sends - Improve media routing with explicit type checks before legacy typeMap The previous implementation caught all image/* content types including image/gif with the generic isImage check, causing them to be sent with replyWithPhoto() which strips animation. Now GIFs are detected first and sent with replyWithAnimation() to preserve their animated nature.
WalkthroughStandardized string quoting and signature formatting across the Telegram module and a targeted refactor in messageManager.ts that replaces async map-based attachment sending with a synchronous for-loop implementing explicit media-type routing and fallbacks. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller
participant MsgMgr as messageManager
participant TgAPI as Telegram API
Caller->>MsgMgr: sendMessageInChunks(message, attachments)
rect rgb(235, 245, 255)
Note over MsgMgr: New flow — sequential processing
loop for each attachment
MsgMgr->>MsgMgr: detectMediaType(attachment)
alt Video
MsgMgr->>TgAPI: sendVideo(...)
TgAPI-->>MsgMgr: success / error
alt error
MsgMgr->>TgAPI: sendDocument(...) // fallback
end
else Animation/GIF
MsgMgr->>TgAPI: sendAnimation(...)
TgAPI-->>MsgMgr: success / error
alt error
MsgMgr->>TgAPI: sendDocument(...) // fallback
end
else Image
MsgMgr->>TgAPI: sendPhoto(...)
TgAPI-->>MsgMgr: success / error
alt error
MsgMgr->>TgAPI: sendMessage(link)
end
else Document or Unknown
MsgMgr->>MsgMgr: getDocumentProcessor(mimeType)
alt processor exists
MsgMgr->>MsgMgr: processDocument(...)
MsgMgr->>TgAPI: sendDocument(...)
else no processor
MsgMgr->>TgAPI: sendMessage(link)
end
end
end
end
MsgMgr-->>Caller: result/ack
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/utils.ts (1)
95-104: Broken link regex — links won’t convert at allThe pattern contains unintended tokens and will never match Markdown links. Replace with a correct bracket/paren matcher.
Apply this diff:
- converted = converted.replace( - /$begin:math:display$([^$end:math:display$]+)]$begin:math:text$([^)]+)$end:math:text$/g, - (_match, text, url) => { - // For link text we escape as plain text. - const formattedText = escapePlainText(text); - const escapedURL = escapeUrl(url); - const formatted = `[${formattedText}](${escapedURL})`; - return storeReplacement(formatted); - }, - ); + converted = converted.replace( + /\[([^\]]+)\]\(([^)]+)\)/g, + (_match, text, url) => { + const formattedText = escapePlainText(text); + const escapedURL = escapeUrl(url); + const formatted = `[${formattedText}](${escapedURL})`; + return storeReplacement(formatted); + }, + );src/service.ts (1)
446-478: Use correct Telegram field for join events (“new_chat_members”).Message updates expose an array new_chat_members, not new_chat_member. Current check won’t run.
Apply this diff:
- if (ctx.message && "new_chat_member" in ctx.message) { - const newMember = ctx.message.new_chat_member as any; - const telegramId = newMember.id.toString(); - const entityId = createUniqueUuid(this.runtime, telegramId) as UUID; + if (ctx.message && "new_chat_members" in ctx.message && Array.isArray((ctx.message as any).new_chat_members)) { + for (const newMember of (ctx.message as any).new_chat_members) { + const telegramId = newMember.id.toString(); + const entityId = createUniqueUuid(this.runtime, telegramId) as UUID; // Skip if we've already synced this entity - if (this.syncedEntityIds.has(telegramId)) return; + if (this.syncedEntityIds.has(telegramId)) continue; // We call ensure connection here for this user. await this.runtime.ensureConnection({ entityId, roomId: roomId, userName: newMember.username, userId: telegramId, name: newMember.first_name || newMember.username || "Unknown User", source: "telegram", channelId: chatId, serverId: chatId, type: ChannelType.GROUP, worldId: worldId, }); - this.syncedEntityIds.add(entityId); + this.syncedEntityIds.add(entityId); + }src/tests.ts (1)
184-191: Set a concrete message_id in the mock.Undefined message_id can ripple into createUniqueUuid and produce inconsistent IDs.
- message: { - message_id: undefined, + message: { + message_id: 1,src/messageManager.ts (1)
926-1009: Fix reaction emoji extraction.You read .type twice; for emoji reactions you need the actual emoji string, not the literal "emoji".
- const reactionType = reaction.new_reaction[0].type; - const reactionEmoji = (reaction.new_reaction[0] as ReactionType).type; // Assuming ReactionType has 'type' for emoji + const r0 = reaction.new_reaction[0] as ReactionType; + const reactionType = r0.type; + const reactionEmoji = + reactionType === "emoji" + ? (r0 as any).emoji + : reactionType === "custom_emoji" + ? (r0 as any).custom_emoji_id + : undefined; ... - text: `Reacted with: ${reactionType === "emoji" ? reactionEmoji : reactionType}`, + text: `Reacted with: ${ + reactionType === "emoji" ? reactionEmoji : reactionType + }`,Optionally enrich memory.metadata with both type and emoji/custom_emoji_id.
🧹 Nitpick comments (9)
src/utils.ts (3)
155-159: Quiet the “RegExp from variable” warning (static, safe here)NULL_CHAR is constant; risk is negligible. To appease tooling and improve clarity, use regex literals.
Apply this diff:
- const NULL_CHAR = String.fromCharCode(0); - const PLACEHOLDER_PATTERN = new RegExp(`(${NULL_CHAR}\\d+${NULL_CHAR})`, "g"); - const PLACEHOLDER_TEST = new RegExp(`^${NULL_CHAR}\\d+${NULL_CHAR}$`); - const PLACEHOLDER_REPLACE = new RegExp(`${NULL_CHAR}(\\d+)${NULL_CHAR}`, "g"); + const NULL_CHAR = "\u0000"; + const PLACEHOLDER_PATTERN = /(\u0000\d+\u0000)/g; + const PLACEHOLDER_TEST = /^\u0000\d+\u0000$/; + const PLACEHOLDER_REPLACE = /\u0000(\d+)\u0000/g;
191-208: splitMessage doesn’t handle a single line > maxLengthA very long line will overflow a chunk. Split oversize lines to respect Telegram’s 4096 limit.
Apply this diff:
export function splitMessage(text: string, maxLength = 4096): string[] { const chunks: string[] = []; if (!text) return chunks; let currentChunk = ""; const lines = text.split("\n"); for (const line of lines) { + // If a single line exceeds maxLength, flush current and hard-split the line. + if (line.length > maxLength) { + if (currentChunk) { + chunks.push(currentChunk); + currentChunk = ""; + } + for (let i = 0; i < line.length; i += maxLength) { + chunks.push(line.slice(i, i + maxLength)); + } + continue; + } if (currentChunk.length + line.length + 1 <= maxLength) { currentChunk += (currentChunk ? "\n" : "") + line; } else { if (currentChunk) chunks.push(currentChunk); currentChunk = line; } } if (currentChunk) chunks.push(currentChunk); return chunks; }
215-241: Button mapping looks fine; consider minimal log content on invalid buttonLogging the full button (includes URLs) might be noisy. Consider logging only the reason/key fields.
src/environment.ts (1)
31-41: Prefer logger and safeParse for validation ergonomicsUse logger.warn for consistency and zod.safeParse to avoid exceptions.
Apply this diff:
-import { z } from "zod"; +import { z } from "zod"; +import { logger } from "@elizaos/core"; @@ -export async function validateTelegramConfig( - runtime: IAgentRuntime, -): Promise<TelegramConfig | null> { +export async function validateTelegramConfig( + runtime: IAgentRuntime, +): Promise<TelegramConfig | null> { @@ - return telegramEnvSchema.parse(config); + const res = telegramEnvSchema.safeParse(config); + if (!res.success) { + const errorMessages = res.error.issues + .map((err) => `${err.path.join(".")}: ${err.message}`) + .join("\n"); + logger.warn(`Telegram configuration validation failed:\n${errorMessages}`); + return null; + } + return res.data; } catch (error) { - if (error instanceof z.ZodError) { - const errorMessages = error.issues - .map((err) => `${err.path.join(".")}: ${err.message}`) - .join("\n"); - console.warn( - `Telegram configuration validation failed:\n${errorMessages}`, - ); - } + logger.warn(`Telegram configuration validation failed: ${String(error)}`); return null; } }src/service.ts (1)
944-951: Minor: avoid deep-clone via JSON for ctx.message.JSON.parse(JSON.stringify(ctx.message)) is heavy and can drop non-enumerables. Use shallow spread or direct access; guards below already handle undefined.
- const replyMessage = JSON.parse(JSON.stringify(ctx.message)); + const replyMessage = ctx.message as any;src/tests.ts (1)
126-165: Add a GIF animation test to prevent regressions.This PR fixes GIF handling; add a test that asserts replyWithAnimation path is used.
Example addition:
+ async testSendingGifAttachment(runtime: IAgentRuntime) { + if (!this.messageManager || !this.bot) throw new Error("Deps not initialized."); + const chat = await this.getChatInfo(runtime); + const mockContext: Partial<Context> = { chat, from: { id: 123, username: "TestUser" } as User, telegram: this.bot.telegram }; + const messageContent = { + text: "Here is a GIF:", + attachments: [ + { + id: "gif-1", + title: "Sample GIF", + source: "https://media.tenor.com/2roX3uxz_68AAAAC/cat-space.gif", + url: "https://media.tenor.com/2roX3uxz_68AAAAC/cat-space.gif", + contentType: "image/gif", + description: "Sample GIF", + }, + ], + } as TelegramContent; + await this.messageManager.sendMessageInChunks(mockContext as Context, messageContent); + logger.success("Message with GIF attachment sent successfully."); + }And register it in this.tests.
src/messageManager.ts (3)
523-556: Remove legacy prefix-based typeMap; route unknowns to safe fallbacks.The PR goal was to eliminate prefix-order pitfalls; keeping a typeMap reintroduces risk (and "doc" prefix is invalid).
- // Fallback: check other media types using legacy typeMap - const typeMap: { [key: string]: MediaType } = { - "image/gif": MediaType.ANIMATION, - doc: MediaType.DOCUMENT, - audio: MediaType.AUDIO, - }; - - let mediaType: MediaType | undefined = undefined; - - for (const prefix in typeMap) { - if (attachment.contentType?.startsWith(prefix)) { - mediaType = typeMap[prefix]; - break; - } - } - - if (mediaType) { - await this.sendMedia( - ctx, - attachment.url, - mediaType, - attachment.description, - ); - } else { - // Final fallback: plain text link with preview enabled - if (ctx.chat) { - await ctx.telegram.sendMessage(ctx.chat.id, url, { - reply_parameters: replyToMessageId - ? { message_id: replyToMessageId } - : undefined, - link_preview_options: { is_disabled: false }, - }); - } - } + // Final fallback: plain text link with preview enabled + if (ctx.chat) { + await ctx.telegram.sendMessage(ctx.chat.id, url, { + reply_parameters: replyToMessageId + ? { message_id: replyToMessageId } + : undefined, + link_preview_options: { is_disabled: false }, + }); + }
416-421: Return the sent messages for attachments, not an empty array.Currently the attachments branch always returns [], preventing memory creation and MESSAGE_SENT parity. Collect and return the results.
- ): Promise<Message.TextMessage[]> { + ): Promise<Message[]> { if (content.attachments && content.attachments.length > 0) { + const sent: Message[] = []; ... - await ctx.replyWithVideo(url, { + const m = await ctx.replyWithVideo(url, { caption: content.text || undefined, supports_streaming: true, reply_parameters: replyToMessageId ? { message_id: replyToMessageId } : undefined, }); + sent.push(m as Message); continue; ... - await ctx.replyWithDocument(url, { + const m2 = await ctx.replyWithDocument(url, { caption: content.text || undefined, reply_parameters: replyToMessageId ? { message_id: replyToMessageId } : undefined, }); + sent.push(m2 as Message); continue; ... - await ctx.replyWithAnimation(url, { + const m = await ctx.replyWithAnimation(url, { caption: content.text || undefined, reply_parameters: replyToMessageId ? { message_id: replyToMessageId } : undefined, }); + sent.push(m as Message); continue; ... - await ctx.replyWithPhoto(url, { + const m = await ctx.replyWithPhoto(url, { caption: content.text || undefined, reply_parameters: replyToMessageId ? { message_id: replyToMessageId } : undefined, }); + sent.push(m as Message); continue; ... - return []; + return sent;Follow‑ups: adjust call sites and types where Message.TextMessage[] is expected to accept Message[].
Also applies to: 426-456, 475-520, 558-567
1030-1071: ChannelType inference in sendMessage defaults to DM.Hardcoding type: "private" risks misclassification for group chats. Pass chat type in or omit channelType here and let upstream infer.
- channelType: getChannelType({ - id: - typeof chatId === "string" - ? Number.parseInt(chatId, 10) - : chatId, - type: "private", // Default to private, will be overridden if in context - } as Chat), + // Defer channelType or add a parameter to supply actual chat type + channelType: undefined as any,Or add an optional chatType param to sendMessage to compute accurately.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (8)
src/constants.ts(1 hunks)src/environment.ts(2 hunks)src/index.ts(1 hunks)src/messageManager.ts(38 hunks)src/service.ts(42 hunks)src/tests.ts(8 hunks)src/types.ts(4 hunks)src/utils.ts(11 hunks)
🧰 Additional context used
🪛 ast-grep (0.39.6)
src/utils.ts
[warning] 155-155: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp((${NULL_CHAR}\\d+${NULL_CHAR}), "g")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 156-156: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^${NULL_CHAR}\\d+${NULL_CHAR}$)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 157-157: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(${NULL_CHAR}(\\d+)${NULL_CHAR}, "g")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔇 Additional comments (7)
src/types.ts (1)
23-54: LGTM — formatting-only changesString literal normalization and minor formatting; no behavioral impact.
Also applies to: 96-98
src/utils.ts (3)
76-85: Markdown conversions — overall goodCode blocks, inline code, headers, and escaping paths look correct and Telegram‑safe.
Also applies to: 88-92, 145-152
1-3: Clean, consistent API surfaceImports and cleanText utility look good.
Also applies to: 255-259
127-134: No explicit Node version constraint in package.json — verify target version supports lookbehindThe repository does not declare an
enginesfield in package.json. While negative lookbehind (used in the regex at lines 127-134) is supported in Node 12.4.0+, and modern dependencies (TypeScript 5.8.3, @types/node@^24.0.10) suggest a practical Node 16+ target, there is no explicit declaration to confirm this.Verify that your minimum supported Node version is 12.4.0 or later, and consider adding an
enginesfield to package.json if this is a published package requiring a specific Node version floor.src/index.ts (1)
1-12: LGTM — formatting-onlyNo behavior change.
src/constants.ts (1)
11-11: LGTM — style consistencyDouble-quoted literal keeps value unchanged; consistent with rest of codebase.
src/messageManager.ts (1)
579-599: No issues found — reply_parameters is correctly supported in Telegraf v7.1.0.Telegraf v7.x properly supports the
reply_parametersobject when passed toctx.telegram.sendMessage, and your implementation matches this pattern. The conditional logic correctly sendsreply_parametersonly for the first message chunk, with proper fallback toundefined.
| const telegramPlugin: Plugin = { | ||
| name: TELEGRAM_SERVICE_NAME, | ||
| description: 'Telegram client plugin', | ||
| description: "Telegram client plugin", | ||
| services: [TelegramService], | ||
| tests: [new TelegramTestSuite()], | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the GIF animation fix is present in messageManager.ts
Confirm explicit GIF routing and the async loop refactor claimed in the PR.
Run:
🏁 Script executed:
#!/bin/bash
# Expect explicit use of replyWithAnimation for GIFs
rg -nP 'replyWithAnimation\s*\(' src/messageManager.ts -C3 || true
# Ensure generic image matcher excludes .gif or contentType image/gif
rg -nP 'gif' src/messageManager.ts -n -C2 || true
# Ensure old prefix-based typeMap removed
rg -nP 'typeMap' src/messageManager.ts -n -C2 || true
# Ensure for-await replaces .map(async)
rg -nP '\.map\s*\(\s*async' src/messageManager.ts -n -C2 || true
rg -nP 'for\s+await\s*\(' src/messageManager.ts -n -C2 || trueLength of output: 1776
GIF routing confirmed; async loop refactor incomplete
The explicit replyWithAnimation usage (line 478) and proper GIF type detection (line 431) are in place. However, the async loop refactor is incomplete: the legacy typeMap fallback (lines 523–535) still exists and hasn't been replaced with for-await. No for-await loop was found in the file. Either complete the refactor by removing the typeMap fallback, or clarify if the fallback is intentional and document why it's retained.
🤖 Prompt for AI Agents
In src/index.ts around lines 431–535, the async loop refactor is incomplete
because the legacy typeMap fallback (lines ~523–535) remains instead of using a
for-await loop; remove the legacy typeMap fallback and replace it with a single
async iterator loop (for await (...) { ... }) that yields the same routing
behavior and preserves the GIF detection and replyWithAnimation behavior already
implemented (ensure you await async handlers, propagate errors appropriately,
and maintain any type checks previously provided by typeMap), or if you
intentionally keep the fallback, add a clear code comment above the fallback
explaining why the legacy path is required and add a unit/integration test
asserting the fallback path is exercised.
- Replace console.error with logger.error for consistency - Fix isImage logic to explicitly exclude GIFs from photo handler - Remove legacy typeMap fallback, use explicit type checks with proper Telegram API methods - Move success log to after bot initialization completes All changes improve code quality, reliability and maintainability while preserving GIF animation support.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/messageManager.ts (1)
512-520: Consider adding error handling to the image branch for consistency.Unlike the video, GIF, document, and audio handlers, the image handler lacks try-catch with fallback logic. If
replyWithPhotofails (e.g., network error, invalid URL, or file too large), the exception will propagate and could disrupt message sending.Consider wrapping in try-catch with a fallback, similar to other media types:
if (isImage) { + try { await ctx.replyWithPhoto(url, { caption: content.text || undefined, reply_parameters: replyToMessageId ? { message_id: replyToMessageId } : undefined, }); continue; + } catch (photoErr) { + // Fallback: send as document or link + try { + await ctx.replyWithDocument(url, { + caption: content.text || undefined, + reply_parameters: replyToMessageId + ? { message_id: replyToMessageId } + : undefined, + }); + continue; + } catch (docErr) { + // Final fallback: send link with preview + if (ctx.chat) { + await ctx.telegram.sendMessage(ctx.chat.id, url, { + reply_parameters: replyToMessageId + ? { message_id: replyToMessageId } + : undefined, + link_preview_options: { is_disabled: false }, + }); + } + continue; + } + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (2)
src/messageManager.ts(37 hunks)src/service.ts(42 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/service.ts
🔇 Additional comments (2)
src/messageManager.ts (2)
427-434: The GIF type detection logic correctly prevents GIFs from being treated as photos.The sequential for-loop replaces the
.map(async)anti-pattern, and the type detection properly handles the root cause:isGifis evaluated first, andisImageuses!isGif &&to exclude GIFs even whencontentTypeis"image/gif". The URL regex forisImagealso explicitly excludes.gifextensions, preventing GIFs from matching when relying on URL-based detection.
476-510: GIF animation support correctly implemented with robust fallbacks.This is the core fix: GIFs now route through
ctx.replyWithAnimation()instead of the photo path, which resolves the reported issue where GIF attachments were sent as static images. The multi-level fallback strategy (animation → document → link with preview) ensures graceful degradation if the primary method fails.
2 participants
Fix: GIF animations not playing in Telegram
Problem
GIF attachments are being sent as static images instead of animations in Telegram messages.
Root Cause
The
sendMessageInChunksmethod uses atypeMapwith prefix matching that can match GIFs to the generic'image'prefix before checking the specific'image/gif'prefix, depending on JavaScript object iteration order. This causes GIFs to be sent viasendMedia()withMediaType.PHOTOinstead ofMediaType.ANIMATION.Solution
ctx.replyWithAnimation()directly for GIFs to preserve animation.giffrom theisImageregex pattern.map(async)withfor awaitloopChanges
isGifcheck beforeisImagereplyWithAnimation()for GIFsTesting
Risk Assessment
LOW RISK - This is a targeted fix that:
Related
Fixes the issue where content with
contentType: 'image/gif'attachments are displayed as static images in Telegram clients.Summary by CodeRabbit
Bug Fixes
Behavior
Style
Note
Fixes GIFs being sent as static images by explicitly routing media (GIF/video/image/audio/document) to correct Telegram APIs with sequential sends, fallbacks, and improved button handling/logging.
replyWithAnimation/replyWithVideo/replyWithPhoto; excludes.giffrom generic image path; adds layered fallbacks (document/link) and enables previews.map(async ...)with awaited loop for attachments; better error handling and logging.Written by Cursor Bugbot for commit 992a250. This will update automatically on new commits. Configure here.