feat: add Telegram channel support#319
Conversation
Add Telegram Bot API integration as a new inbox channel, enabling agents to receive and reply to messages from Telegram users. - Telegram bot webhook endpoint for receiving messages - Send text, photos, documents, video, audio, voice, stickers - Outgoing messages with attachments sent via multipart upload - Contact creation with external_user_id (telegram user ID) - Profile photo download and storage as contact avatar - Conversation grouping by contact per inbox (open conversations) - Telegram chat_id stored in message meta for reply routing - Bot token encrypted at rest using app encryption key - DB migration v2.3.0 adds 'telegram' to channels enum - Admin UI: create/edit Telegram inbox with bot token config - Sidebar: display Telegram username with t.me link - Contact views: strip 'telegram_' prefix from external ID display
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughAdds end-to-end Telegram inbox support: DB enum + migration, Telegram API types, channel implementation (send/receive, media, webhook), inbox config/encryption, conversation/contact routing and queries, HTTP webhook endpoint, and admin UI with i18n. ChangesTelegram Inbox Implementation
🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/inboxes.go`:
- Around line 472-474: The validation currently only checks cfg.BotToken == ""
and therefore accepts whitespace-only values; change the check to trim
whitespace before validating (use strings.TrimSpace(cfg.BotToken) == "") and
return the same envelope.NewError(envelope.InputError,
app.i18n.Ts("globals.messages.empty", "name", "bot_token"), nil) when the
trimmed value is empty so whitespace-only tokens are rejected; ensure you import
strings if not already imported.
In `@cmd/init.go`:
- Around line 748-755: The code currently reuses the shared ko variable and
calls ko.Load(...) which merges state across inboxes; create a fresh local Koanf
instance for each inbox (e.g., instantiate a new koanf via koanf.New(...) or
equivalent) and call that instance's Load(...) and UnmarshalWithConf(...)
against inboxRecord.Config so the parsing is isolated; apply the same change
where the pattern appears in initEmailInbox and initLiveChatInbox so each
function creates and uses its own local Koanf instead of the shared ko.
In `@cmd/telegram.go`:
- Around line 13-47: The webhook handler currently accepts unauthenticated
updates; update handleTelegramWebhook to validate the
X-Telegram-Bot-Api-Secret-Token header against a stored secret on the Telegram
inbox before decoding or processing the update: add a secret field (e.g.
WebhookSecret or SecretToken) to the telegram.Telegram struct when the inbox is
created, then in handleTelegramWebhook, after retrieving inb and type-asserting
to *telegram.Telegram (tgInbox), read the header
(RequestCtx.Request.Header.Peek("X-Telegram-Bot-Api-Secret-Token") or
equivalent), compare it to tgInbox.WebhookSecret, and if it’s missing or does
not match return an unauthorized/forbidden status and stop processing; log
mismatches and only call tgInbox.ProcessWebhookUpdate(update) when the token
check passes.
- Around line 16-20: The parsing of the "id" path parameter can panic because
r.RequestCtx.UserValue("id") may be nil or not a string; update the inbox ID
parsing around inboxID and the call to r.RequestCtx.UserValue("id") to first
retrieve the value into a variable, check for nil, perform a safe type assertion
(e.g., s, ok := value.(string)) and also handle empty string, then call
strconv.Atoi on that string and return a 400 (fasthttp.StatusBadRequest) on any
failure; modify the block that currently does inboxID, err :=
strconv.Atoi(r.RequestCtx.UserValue("id").(string)) to implement these defensive
checks.
In `@frontend/apps/main/src/features/admin/inbox/TelegramInboxForm.vue`:
- Around line 172-174: Replace the generic saved-success toast with the
copy-specific message: in the emitter.emit call that uses
EMITTER_EVENTS.SHOW_TOAST, change the description from
t('globals.messages.savedSuccessfully') to the copy toast key
t('globals.messages.copied') so the toast reflects a clipboard/copy action.
In `@frontend/apps/main/src/views/admin/inbox/NewInbox.vue`:
- Line 114: Replace the hardcoded title string in NewInbox.vue (the "title:
'Telegram'" entry) with a translated key using the app's i18n helper (e.g., $t
or i18n.t) and add the corresponding translation key (for example
"channels.telegram") to the locale files for all supported languages; update any
places that construct the inbox/channel metadata (the object with title) to use
that translation call so the UI shows localized text.
In `@frontend/apps/main/src/views/contact/ContactDetailView.vue`:
- Around line 190-196: The computed telegramUsername currently always returns ''
so the Telegram link never renders; update the computed (telegramUsername) to
actually derive a username by first checking known fields such as
contact.value.custom_attributes['telegram_username'] and
contact.value.meta?.telegram?.username, then fallback to parsing
contact.value.external_user_id when it startsWith('telegram_') by returning the
substring after 'telegram_'; if none exist, return ''. Alternatively, if backend
won’t provide any Telegram data yet, remove the Telegram link block instead of
keeping a dead computed value.
In `@internal/conversation/message.go`:
- Around line 1172-1204: The Telegram branch that calls
findOpenConversationForContact and then CreateConversation is racy; make the
lookup+create atomic by performing both inside a DB-protected critical section:
acquire a per-(contact.ID,in.InboxID) lock (e.g., PostgreSQL advisory lock) or
start a transaction and SELECT FOR UPDATE on a row/key representing the
contact+inbox, then re-check for an open conversation via
findOpenConversationForContact and only call CreateConversation if none exists;
alternatively implement a DB uniqueness constraint for an “open conversation per
contact+inbox” and use an upsert/INSERT ... ON CONFLICT to return the existing
conversation UUID/ID instead of creating duplicates. Ensure you change the logic
around findOpenConversationForContact and CreateConversation to run inside this
locked/transactional scope and handle returning the existing conversation when
detected.
In `@internal/inbox/channel/telegram/telegram.go`:
- Around line 68-75: The current code short-circuits sends by returning nil when
message.MessageReceiverID <= 0, causing Telegram replies (which rely on
t.extractChatID from message.Meta) to be dropped; remove or change the early
return so the code always attempts to resolve the Telegram chat id via
t.extractChatID(message.Meta) (and only error/return if chatID is empty),
ensuring the transport does not silently succeed when MessageReceiverID is
unset.
- Around line 82-92: The current loop over message.Attachments calls
t.sendAttachment and logs errors but always returns nil, hiding failures; modify
the attachment-send block (the loop using message.Attachments and the
t.sendAttachment call) to collect any send errors (e.g., into a slice or a
single aggregated error) and after the loop return a non-nil error when any
attachment upload failed (or wrap multiple errors into one), ensuring the
function returns success only when all attachments were sent successfully.
- Around line 143-165: The code dereferences msg.From and msg.Chat without nil
checks when building models.IncomingContact, calling
t.downloadUserAvatar(msg.From.ID), computing content, and marshaling meta; add
explicit nil guards: check if msg.From != nil before reading
FirstName/LastName/Username/ID and before calling downloadUserAvatar or building
ExternalUserID, and check if msg.Chat != nil before using msg.Chat.ID; if either
is nil, handle gracefully (return an error, log and ignore the message, or set
safe defaults) so the webhook does not panic (apply these checks around the
IncomingContact construction, the avatar download call, the content selection,
and the meta map creation).
- Around line 120-134: The SetWebhook implementation (Telegram.SetWebhook) must
include the Telegram secret token by adding a "secret_token" field to the JSON
payload when calling setWebhook; update the payload construction to include
t.config.SecretToken (or whatever field holds the secret) so Telegram will send
the token on webhook deliveries. In the public webhook handler
(handleTelegramWebhook) validate the incoming request header
"X-Telegram-Bot-Api-Secret-Token" against the same configured secret before
calling ProcessWebhookUpdate; if the header is missing or does not match, return
an unauthorized response (HTTP 401) and do not forward to ProcessWebhookUpdate.
Ensure both sides use the same config value and treat token comparison as a
constant-time equality for safety where possible.
In `@internal/inbox/inbox.go`:
- Around line 423-444: The update branch for "telegram" should preserve the real
bot token when the incoming config carries the masked password dummy produced by
Inbox.ClearPasswords() (not just when empty); modify the logic in the case
"telegram" block to treat updateCfg["bot_token"] as "unchanged" if it is
missing, empty, or equals the password dummy used by Inbox.ClearPasswords(), and
in those cases set updateCfg["bot_token"] = currentCfg["bot_token"] before
marshalling; reference the existing variables updateCfg and currentCfg and the
Inbox.ClearPasswords() dummy value when implementing the comparison so the dummy
is not encrypted and the real token is retained.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2ec52868-74ff-4d47-8c02-75cf0eaa1610
📒 Files selected for processing (22)
cmd/handlers.gocmd/inboxes.gocmd/init.gocmd/telegram.gocmd/upgrade.gofrontend/apps/main/src/features/admin/inbox/TelegramInboxForm.vuefrontend/apps/main/src/features/contact/ContactsList.vuefrontend/apps/main/src/features/conversation/sidebar/ConversationSideBarContact.vuefrontend/apps/main/src/views/admin/inbox/EditInbox.vuefrontend/apps/main/src/views/admin/inbox/NewInbox.vuefrontend/apps/main/src/views/contact/ContactDetailView.vuei18n/en.jsoninternal/conversation/conversation.gointernal/conversation/message.gointernal/conversation/models/models.gointernal/conversation/queries.sqlinternal/inbox/channel/telegram/telegram.gointernal/inbox/channel/telegram/types.gointernal/inbox/inbox.gointernal/inbox/models/models.gointernal/migrations/v2.3.0.goschema.sql
| // Load JSON data into Koanf. | ||
| if err := ko.Load(rawbytes.Provider([]byte(inboxRecord.Config)), kjson.Parser()); err != nil { | ||
| return nil, fmt.Errorf("loading config: %w", err) | ||
| } | ||
|
|
||
| if err := ko.UnmarshalWithConf("", &config, koanf.UnmarshalConf{Tag: "json"}); err != nil { | ||
| return nil, fmt.Errorf("unmarshalling `%s` %s config: %w", inboxRecord.Channel, inboxRecord.Name, err) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's see the actual code at lines 748-755 and surrounding context
fd cmd/init.go | head -1 | xargs -I {} bash -c 'wc -l {} && cat -n {} | sed -n "740,760p"'Repository: abhinavxd/libredesk
Length of output: 45
🏁 Script executed:
# Let's also search for the function definition and related code
rg -n "initTelegramInbox" cmd/init.go -A 20 -B 2Repository: abhinavxd/libredesk
Length of output: 2083
🏁 Script executed:
# Check where the global `ko` is declared
rg -n "^var.*ko|:= koanf\.|ko\s*=" cmd/init.go | head -20Repository: abhinavxd/libredesk
Length of output: 45
🏁 Script executed:
# Look for other inbox initialization functions to see if they have similar patterns
rg -n "func init.*Inbox" cmd/init.goRepository: abhinavxd/libredesk
Length of output: 576
🏁 Script executed:
# Look for ko variable declaration at package level in cmd/init.go
rg -n "^var.*ko|^ko\s*:=" cmd/init.go | head -10Repository: abhinavxd/libredesk
Length of output: 45
🏁 Script executed:
# Check initEmailInbox implementation
rg -n "func initEmailInbox" cmd/init.go -A 40Repository: abhinavxd/libredesk
Length of output: 1871
🏁 Script executed:
# Check initLiveChatInbox implementation
rg -n "func initLiveChatInbox" cmd/init.go -A 30Repository: abhinavxd/libredesk
Length of output: 1407
🏁 Script executed:
# Search for all usages of 'ko' in the file to understand the full pattern
rg -n "\bko\." cmd/init.goRepository: abhinavxd/libredesk
Length of output: 4584
🏁 Script executed:
# Check for package-level variable declarations at the beginning of the file
head -100 cmd/init.go | cat -n | grep -E "var|const"Repository: abhinavxd/libredesk
Length of output: 391
🏁 Script executed:
# Search for ko declaration - look at the very beginning of the file
head -200 cmd/init.go | cat -nRepository: abhinavxd/libredesk
Length of output: 8974
🏁 Script executed:
# Look for ko declaration more broadly - might be before line 74
rg -n "^var\s+ko\b|^ko\s*:=\s*koanf" cmd/init.goRepository: abhinavxd/libredesk
Length of output: 45
🏁 Script executed:
# Check imports to understand Koanf source
head -50 cmd/init.go | grep -i koanfRepository: abhinavxd/libredesk
Length of output: 45
🏁 Script executed:
# Look for global ko variable - check around main function and beyond line 200
tail -n +200 cmd/init.go | head -100 | cat -nRepository: abhinavxd/libredesk
Length of output: 3454
🏁 Script executed:
# Better approach - search for "var" at the very file level (no indentation)
rg "^var\s" cmd/init.goRepository: abhinavxd/libredesk
Length of output: 45
🏁 Script executed:
# Check the main function to see how ko is created/used
rg -n "func main" cmd/init.go -A 50Repository: abhinavxd/libredesk
Length of output: 45
🏁 Script executed:
# Look for ko usage without seeing declaration - it might be in a different file
# Search for "var ko" across all Go files
fd "\.go$" | xargs rg "^var\s+ko\b" 2>/dev/nullRepository: abhinavxd/libredesk
Length of output: 47
🏁 Script executed:
# Or check all files in cmd directory
fd "\.go$" cmd/ | xargs rg "var\s+ko\s*\*koanf" 2>/dev/nullRepository: abhinavxd/libredesk
Length of output: 47
🏁 Script executed:
# Let's search the entire codebase for where ko is initialized
rg "ko\s*=\s*koanf\.New" --type goRepository: abhinavxd/libredesk
Length of output: 105
🏁 Script executed:
# Alternative: look at the main function location
find . -name "*.go" -exec rg -l "func main" {} \;Repository: abhinavxd/libredesk
Length of output: 77
🏁 Script executed:
# Found it! ko is in cmd/main.go. Let's see the declaration and context
rg -n "ko\s*=" cmd/main.go -B 5 -A 5 | head -30Repository: abhinavxd/libredesk
Length of output: 331
🏁 Script executed:
# Now let's understand Koanf's Load() behavior by checking if there are any tests or comments
# in the repository that show how Load() works with multiple calls
rg -n "ko\.Load" cmd/init.go | head -20Repository: abhinavxd/libredesk
Length of output: 666
🌐 Web query:
Koanf Load() method behavior accumulate merge or replace state
💡 Result:
The Load method in Koanf is designed to accumulate configuration state by merging new data into the existing configuration [1][2]. It does not replace the entire state [1][2]. When you call Load, Koanf performs a recursive merge of the new configuration map into the current internal state [1][2]. By default, this process merges nested maps (map[string]any) while overwriting static values such as strings, integers, or slices [1][2]. Key behaviors of this accumulation process include: 1. Incremental Merging: Every successive call to Load or Merge merges new configuration data into the existing set [1][2]. This allows you to layer configurations from multiple sources (e.g., loading a base YAML file followed by environment variable overrides) in any order [1][2]. 2. Recursive Merging: For nested structures, Koanf traverses the maps and merges them recursively rather than simply replacing the top-level key [1][2]. 3. Strict Merge Option: If the default merging behavior is not desired, you can enable StrictMerge in the Koanf configuration [1][2]. When enabled, Load will return an error if it detects conflicting keys or types during the merge process [1][2]. 4. Customization: You can further control how data is merged by providing a custom merge function using the WithMergeFunc option [1][2]. If you need to clear the existing state and start fresh, you must create a new Koanf instance (e.g., k = koanf.New(".")) rather than relying on Load to reset the state [3].
Citations:
- 1: https://github.com/knadh/koanf
- 2: https://pkg.go.dev/github.com/knadh/koanf
- 3: https://github.com/knadh/koanf/blob/master/README.md?plain=1
🏁 Script executed:
# Final verification: confirm the sequence of calls to these functions to ensure they're truly sequential
rg -n "initEmailInbox|initLiveChatInbox|initTelegramInbox" cmd/init.goRepository: abhinavxd/libredesk
Length of output: 908
🏁 Script executed:
# Check how makeInboxInitializer is used to confirm multiple inboxes are initialized in sequence
rg -n "makeInboxInitializer\|InitInboxes" cmd/init.go -B 2 -A 5Repository: abhinavxd/libredesk
Length of output: 45
Use an isolated Koanf instance for per-inbox config parsing to prevent cross-inbox state bleed.
Loading each inbox's JSON configuration into the global ko causes accumulation across sequential inbox initializations. Koanf's Load() merges state rather than replacing it, so configs from previously loaded inboxes remain in memory and can contaminate subsequent inbox parsing. Parse each inbox config with a fresh local Koanf instance.
Proposed fix
func initTelegramInbox(inboxRecord imodels.Inbox, msgStore inbox.MessageStore, usrStore inbox.UserStore) (inbox.Inbox, error) {
var config telegram.Config
+ k := koanf.New(".")
// Load JSON data into Koanf.
- if err := ko.Load(rawbytes.Provider([]byte(inboxRecord.Config)), kjson.Parser()); err != nil {
+ if err := k.Load(rawbytes.Provider([]byte(inboxRecord.Config)), kjson.Parser()); err != nil {
return nil, fmt.Errorf("loading config: %w", err)
}
- if err := ko.UnmarshalWithConf("", &config, koanf.UnmarshalConf{Tag: "json"}); err != nil {
+ if err := k.UnmarshalWithConf("", &config, koanf.UnmarshalConf{Tag: "json"}); err != nil {
return nil, fmt.Errorf("unmarshalling `%s` %s config: %w", inboxRecord.Channel, inboxRecord.Name, err)
}Apply the same fix to initEmailInbox (line 654) and initLiveChatInbox (line 716), which have the identical pattern.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Load JSON data into Koanf. | |
| if err := ko.Load(rawbytes.Provider([]byte(inboxRecord.Config)), kjson.Parser()); err != nil { | |
| return nil, fmt.Errorf("loading config: %w", err) | |
| } | |
| if err := ko.UnmarshalWithConf("", &config, koanf.UnmarshalConf{Tag: "json"}); err != nil { | |
| return nil, fmt.Errorf("unmarshalling `%s` %s config: %w", inboxRecord.Channel, inboxRecord.Name, err) | |
| } | |
| k := koanf.New(".") | |
| // Load JSON data into Koanf. | |
| if err := k.Load(rawbytes.Provider([]byte(inboxRecord.Config)), kjson.Parser()); err != nil { | |
| return nil, fmt.Errorf("loading config: %w", err) | |
| } | |
| if err := k.UnmarshalWithConf("", &config, koanf.UnmarshalConf{Tag: "json"}); err != nil { | |
| return nil, fmt.Errorf("unmarshalling `%s` %s config: %w", inboxRecord.Channel, inboxRecord.Name, err) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/init.go` around lines 748 - 755, The code currently reuses the shared ko
variable and calls ko.Load(...) which merges state across inboxes; create a
fresh local Koanf instance for each inbox (e.g., instantiate a new koanf via
koanf.New(...) or equivalent) and call that instance's Load(...) and
UnmarshalWithConf(...) against inboxRecord.Config so the parsing is isolated;
apply the same change where the pattern appears in initEmailInbox and
initLiveChatInbox so each function creates and uses its own local Koanf instead
of the shared ko.
|
Hey I tried it locally it works, stickers were not working. The field |
…telegram
- Add webhook authentication using X-Telegram-Bot-Api-Secret-Token header
(secret derived from bot token via HMAC-SHA256)
- Fix animated/video stickers by downloading thumbnail instead of .tgs/.webm
- Stop using external_user_id field for Telegram user identification;
use synthetic email format {telegram_user_id}@telegram instead
- Revert external_user_id display changes in contact views
There was a problem hiding this comment.
🧹 Nitpick comments (3)
internal/inbox/channel/telegram/telegram.go (3)
137-142: 💤 Low valueEmpty secret token bypasses webhook authentication.
VerifyWebhookreturnstruewhensecretTokenis empty, allowing unauthenticated requests through. While this provides backwards compatibility for webhooks set up withoutsecret_token, it also means anyone who discovers the webhook URL can inject arbitrary messages by omitting the header.Consider logging a warning when verification is skipped due to empty token, or deprecating the compatibility path after a migration period.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/inbox/channel/telegram/telegram.go` around lines 137 - 142, The VerifyWebhook function currently returns true if the incoming secretToken is empty, allowing unauthenticated requests; update Telegram.VerifyWebhook to not silently bypass authentication: if secretToken == "" then emit a warning via your logger (e.g., process or package logger) indicating verification was skipped due to empty token and either return false or add a controlled compatibility toggle so requests without a token are rejected by default; ensure the comparison still uses t.WebhookSecret() for normal verification and reference the Telegram.VerifyWebhook and Telegram.WebhookSecret() symbols when making the change.
304-307: 💤 Low valueEmpty branch flagged by static analysis.
The empty else block with only a comment doesn't affect functionality, but could be clearer. Consider logging when no thumbnail is available for animated stickers, or removing the else block entirely if no action is needed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/inbox/channel/telegram/telegram.go` around lines 304 - 307, Remove the empty else branch that only contains the comment or replace it with a concise log statement so the code is not an empty branch; locate the block handling animated-sticker thumbnails (the branch that currently contains the comment "// No thumbnail available, set emoji as content placeholder.") and either delete the else { ... } entirely or call the existing logger (e.g., processLogger or logger used in this file) to log a debug/info message like "no thumbnail for animated sticker, using emoji placeholder" so the branch is meaningful.
526-553: 💤 Low valueUnchecked error returns in multipart construction.
writer.WriteField(lines 530, 532) andpart.Write(line 539) return errors that are ignored. While writes to abytes.Bufferrarely fail, checking errors is defensive and aligns with Go best practices.Proposed fix
func (t *Telegram) uploadMultipart(apiURL, chatID, fieldName string, content []byte, fileName, caption string) error { body := &bytes.Buffer{} writer := multipart.NewWriter(body) - writer.WriteField("chat_id", chatID) + if err := writer.WriteField("chat_id", chatID); err != nil { + return fmt.Errorf("writing chat_id field: %w", err) + } if caption != "" { - writer.WriteField("caption", caption) + if err := writer.WriteField("caption", caption); err != nil { + return fmt.Errorf("writing caption field: %w", err) + } } part, err := writer.CreateFormFile(fieldName, fileName) if err != nil { return fmt.Errorf("creating form file: %w", err) } - part.Write(content) - writer.Close() + if _, err := part.Write(content); err != nil { + return fmt.Errorf("writing file content: %w", err) + } + if err := writer.Close(); err != nil { + return fmt.Errorf("closing multipart writer: %w", err) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/inbox/channel/telegram/telegram.go` around lines 526 - 553, In uploadMultipart you currently ignore errors from writer.WriteField and part.Write (and writer.Close), so update the function to check and return wrapped errors for each write/close: capture the error returned by writer.WriteField("chat_id", ...), writer.WriteField("caption", ...) and the io.Writer returned by writer.CreateFormFile when calling part.Write(content), and also check writer.Close() before making t.httpClient.Post; return fmt.Errorf with context (e.g., "writing field chat_id: %w", "writing caption: %w", "writing form file: %w", "closing multipart writer: %w") so failures are propagated instead of ignored.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/inbox/channel/telegram/telegram.go`:
- Around line 137-142: The VerifyWebhook function currently returns true if the
incoming secretToken is empty, allowing unauthenticated requests; update
Telegram.VerifyWebhook to not silently bypass authentication: if secretToken ==
"" then emit a warning via your logger (e.g., process or package logger)
indicating verification was skipped due to empty token and either return false
or add a controlled compatibility toggle so requests without a token are
rejected by default; ensure the comparison still uses t.WebhookSecret() for
normal verification and reference the Telegram.VerifyWebhook and
Telegram.WebhookSecret() symbols when making the change.
- Around line 304-307: Remove the empty else branch that only contains the
comment or replace it with a concise log statement so the code is not an empty
branch; locate the block handling animated-sticker thumbnails (the branch that
currently contains the comment "// No thumbnail available, set emoji as content
placeholder.") and either delete the else { ... } entirely or call the existing
logger (e.g., processLogger or logger used in this file) to log a debug/info
message like "no thumbnail for animated sticker, using emoji placeholder" so the
branch is meaningful.
- Around line 526-553: In uploadMultipart you currently ignore errors from
writer.WriteField and part.Write (and writer.Close), so update the function to
check and return wrapped errors for each write/close: capture the error returned
by writer.WriteField("chat_id", ...), writer.WriteField("caption", ...) and the
io.Writer returned by writer.CreateFormFile when calling part.Write(content),
and also check writer.Close() before making t.httpClient.Post; return fmt.Errorf
with context (e.g., "writing field chat_id: %w", "writing caption: %w", "writing
form file: %w", "closing multipart writer: %w") so failures are propagated
instead of ignored.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 797acd79-bd8c-4828-aabe-eec1be928be7
📒 Files selected for processing (4)
cmd/telegram.gofrontend/apps/main/src/features/conversation/sidebar/ConversationSideBarContact.vueinternal/inbox/channel/telegram/telegram.gointernal/inbox/channel/telegram/types.go
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/apps/main/src/features/conversation/sidebar/ConversationSideBarContact.vue
- Trim bot_token before validation to reject whitespace-only values - Use isolated Koanf instance in initTelegramInbox to prevent cross-inbox state bleed - Add defensive nil check for msg.From and msg.Chat pointers - Defensive parsing for webhook path parameter to prevent panic - Return last error from attachment sends instead of silently succeeding - Remove MessageReceiverID short-circuit (Telegram routes via chat_id in meta) - Preserve masked bot_token (password dummy) on inbox update - Remove duplicate ExternalUserID assignment - Fix copy toast to use 'copied' message key
527b6a0 to
b509ee2
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/telegram.go`:
- Around line 58-63: When tgInbox.ProcessWebhookUpdate(update) returns an error,
do not unconditionally return 200—log the error as you already do
(app.lo.Error("telegram webhook: error processing update", "error", err)) but
also set a non-2xx retriable status (e.g.,
r.RequestCtx.SetStatusCode(fasthttp.StatusInternalServerError)) and return the
error so the HTTP handler does not treat it as success; update the handler code
around tgInbox.ProcessWebhookUpdate, app.lo.Error, and
r.RequestCtx.SetStatusCode to implement this behavior so Telegram will retry
using the SourceID-based dedupe.
In `@internal/inbox/channel/telegram/telegram.go`:
- Around line 296-309: When handling animated/video stickers in the msg.Sticker
branch (inside the IsAnimated || IsVideo check) you must not leave the path that
has no Thumbnail empty; update that branch (where Thumbnail == nil) to add a
fallback so ProcessWebhookUpdate doesn't persist an empty message — either
create and append a small text attachment/placeholder to attachments or set the
message content to msg.Sticker.Emoji (if non-empty) or a literal "[sticker]"
placeholder so that attachments or content exist for ProcessWebhookUpdate to
persist; modify the code around t.downloadFile/msg.Sticker and attachments to
implement this fallback.
- Around line 135-139: VerifyWebhook currently accepts empty incoming
secretToken, allowing spoofed requests; change it so an empty incoming token is
only accepted if the local webhook secret is also empty. Update the
VerifyWebhook function to return true only when both secretToken == "" and
t.WebhookSecret() == ""; otherwise require secretToken == t.WebhookSecret().
Reference the VerifyWebhook method and the WebhookSecret accessor (and
SetWebhook behavior that registers a secret_token) and ensure
X-Telegram-Bot-Api-Secret-Token must be present and match when a local secret
exists.
- Around line 486-490: The payload currently sets "parse_mode":"HTML" while
passing raw text (see the payload creation around variables payload, chatID,
text), which will cause Telegram parse errors for characters like &, <, >;
either remove the "parse_mode" key so messages are sent as plain text or
HTML-escape the text before marshaling. To fix: update the payload construction
in telegram.go to omit "parse_mode" when sending agent text, or run the text
through an HTML-escape helper (escape &, <, > to &, <, >) and then
keep "parse_mode":"HTML"; ensure you update where payload is json.Marshal'ed so
the chat_id/text pair uses the chosen approach consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: cd4bcaee-dad5-4315-8e31-28193d443474
📒 Files selected for processing (7)
.gitignorecmd/inboxes.gocmd/init.gocmd/telegram.gofrontend/apps/main/src/features/admin/inbox/TelegramInboxForm.vueinternal/inbox/channel/telegram/telegram.gointernal/inbox/inbox.go
🚧 Files skipped from review as they are similar to previous changes (4)
- frontend/apps/main/src/features/admin/inbox/TelegramInboxForm.vue
- cmd/inboxes.go
- internal/inbox/inbox.go
- cmd/init.go
| if err := tgInbox.ProcessWebhookUpdate(update); err != nil { | ||
| app.lo.Error("telegram webhook: error processing update", "error", err) | ||
| } | ||
|
|
||
| r.RequestCtx.SetStatusCode(fasthttp.StatusOK) | ||
| return nil |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "telegram.go" -type fRepository: abhinavxd/libredesk
Length of output: 127
🏁 Script executed:
head -70 cmd/telegram.go | tail -20Repository: abhinavxd/libredesk
Length of output: 726
🏁 Script executed:
rg "ProcessWebhookUpdate" --type go -B 3 -A 3Repository: abhinavxd/libredesk
Length of output: 774
🏁 Script executed:
rg "handleTelegramWebhook" --type go -B 5 -A 15Repository: abhinavxd/libredesk
Length of output: 2678
🏁 Script executed:
rg "func.*ProcessWebhookUpdate" --type go -A 30 internal/inbox/channel/telegram/Repository: abhinavxd/libredesk
Length of output: 2234
🏁 Script executed:
rg "idempoten|duplicate|retry" --type go cmd/telegram.go internal/inbox/channel/telegram/Repository: abhinavxd/libredesk
Length of output: 45
🏁 Script executed:
cat -n internal/inbox/channel/telegram/telegram.go | head -100Repository: abhinavxd/libredesk
Length of output: 2957
🏁 Script executed:
rg "func.*ProcessWebhookUpdate" --type go -A 50 internal/inbox/channel/telegram/telegram.goRepository: abhinavxd/libredesk
Length of output: 1559
🏁 Script executed:
rg "messageStore\.|userStore\." --type go internal/inbox/channel/telegram/telegram.go | head -20Repository: abhinavxd/libredesk
Length of output: 132
🏁 Script executed:
web_search
Telegram bot webhook HTTP status codes retry behaviorRepository: abhinavxd/libredesk
Length of output: 161
🌐 Web query:
Telegram bot webhook HTTP status codes retry behavior 2xx vs 5xx
💡 Result:
The Telegram Bot API handles webhook delivery based on the HTTP status code returned by your server [1]. If your server responds with a 2xx status code (e.g., 200 OK), Telegram considers the update successfully delivered [1][2]. If your server responds with any status code outside the 2xx range (such as 5xx or 4xx errors), Telegram considers the request unsuccessful [1]. In these cases, Telegram will automatically retry the delivery of the update [1]. The system will continue to attempt redelivery for a reasonable number of attempts before eventually giving up on that specific update [1]. To ensure reliable operation, your webhook listener should be configured to return a 2xx status code immediately upon receiving and successfully processing (or queuing for processing) an update [3][2]. If your server returns non-2xx codes, it will trigger these automatic retries, which can lead to duplicate processing if your logic is not idempotent [1]. You can monitor the status of your webhook, including the last error message and date, using the getWebhookInfo method [1].
Citations:
- 1: https://core.telegram.org/bots/api
- 2: https://stackoverflow.com/questions/64602529/how-do-i-say-to-the-telegram-api-that-i-got-their-webhook-request-successfully
- 3: https://www.alchemy.com/support/why-am-i-getting-5xx-errors-repeatedly-when-setting-up-a-webhook
🏁 Script executed:
rg "func.*ProcessWebhookUpdate" --type go -A 60 internal/inbox/channel/telegram/telegram.go | tail -20Repository: abhinavxd/libredesk
Length of output: 671
🏁 Script executed:
rg "EnqueueIncoming" --type go -B 2 -A 5Repository: abhinavxd/libredesk
Length of output: 2164
Return a retriable status on processing failures.
If ProcessWebhookUpdate() returns an error (e.g., message enqueue fails), the update was not durably accepted, but the handler still replies 200 OK. Telegram only retries on non-2xx responses; returning success here causes message loss on transient failures. The SourceID field uniquely identifies each message (telegram_{chat_id}_{message_id}), so returning 500 Internal Server Error on processing failures allows safe retries without duplicate handling.
Return a retriable status on processing failures
if err := tgInbox.ProcessWebhookUpdate(update); err != nil {
app.lo.Error("telegram webhook: error processing update", "error", err)
+ r.RequestCtx.SetStatusCode(fasthttp.StatusInternalServerError)
+ return nil
}
r.RequestCtx.SetStatusCode(fasthttp.StatusOK)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if err := tgInbox.ProcessWebhookUpdate(update); err != nil { | |
| app.lo.Error("telegram webhook: error processing update", "error", err) | |
| } | |
| r.RequestCtx.SetStatusCode(fasthttp.StatusOK) | |
| return nil | |
| if err := tgInbox.ProcessWebhookUpdate(update); err != nil { | |
| app.lo.Error("telegram webhook: error processing update", "error", err) | |
| r.RequestCtx.SetStatusCode(fasthttp.StatusInternalServerError) | |
| return nil | |
| } | |
| r.RequestCtx.SetStatusCode(fasthttp.StatusOK) | |
| return nil |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/telegram.go` around lines 58 - 63, When
tgInbox.ProcessWebhookUpdate(update) returns an error, do not unconditionally
return 200—log the error as you already do (app.lo.Error("telegram webhook:
error processing update", "error", err)) but also set a non-2xx retriable status
(e.g., r.RequestCtx.SetStatusCode(fasthttp.StatusInternalServerError)) and
return the error so the HTTP handler does not treat it as success; update the
handler code around tgInbox.ProcessWebhookUpdate, app.lo.Error, and
r.RequestCtx.SetStatusCode to implement this behavior so Telegram will retry
using the SourceID-based dedupe.
| func (t *Telegram) VerifyWebhook(secretToken string) bool { | ||
| if secretToken == "" { | ||
| return true | ||
| } | ||
| return secretToken == t.WebhookSecret() |
There was a problem hiding this comment.
Reject missing Telegram secret headers.
SetWebhook() always registers a secret_token, but VerifyWebhook("") still returns true. That means the public webhook accepts spoofed requests as long as the caller omits X-Telegram-Bot-Api-Secret-Token. Only skip verification when no local secret exists; otherwise require an exact match.
Minimal fix
+import "crypto/subtle"
+
func (t *Telegram) VerifyWebhook(secretToken string) bool {
- if secretToken == "" {
- return true
- }
- return secretToken == t.WebhookSecret()
+ expected := t.WebhookSecret()
+ if expected == "" {
+ return secretToken == ""
+ }
+ return subtle.ConstantTimeCompare([]byte(secretToken), []byte(expected)) == 1
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/inbox/channel/telegram/telegram.go` around lines 135 - 139,
VerifyWebhook currently accepts empty incoming secretToken, allowing spoofed
requests; change it so an empty incoming token is only accepted if the local
webhook secret is also empty. Update the VerifyWebhook function to return true
only when both secretToken == "" and t.WebhookSecret() == ""; otherwise require
secretToken == t.WebhookSecret(). Reference the VerifyWebhook method and the
WebhookSecret accessor (and SetWebhook behavior that registers a secret_token)
and ensure X-Telegram-Bot-Api-Secret-Token must be present and match when a
local secret exists.
| if msg.Sticker != nil { | ||
| // For animated (.tgs) or video (.webm) stickers, use the thumbnail if available. | ||
| // Static stickers (.webp) can be downloaded directly. | ||
| if msg.Sticker.IsAnimated || msg.Sticker.IsVideo { | ||
| if msg.Sticker.Thumbnail != nil { | ||
| if att, err := t.downloadFile(msg.Sticker.Thumbnail.FileID, "sticker.jpg", "image/jpeg"); err == nil { | ||
| attachments = append(attachments, att) | ||
| } else { | ||
| t.lo.Error("error downloading telegram sticker thumbnail", "error", err) | ||
| } | ||
| } else { | ||
| // No thumbnail available, set emoji as content placeholder. | ||
| // The message content will show the emoji. | ||
| } |
There was a problem hiding this comment.
Sticker-only updates can still be persisted blank.
When an animated/video sticker has no thumbnail, this branch neither downloads media nor sets a fallback. ProcessWebhookUpdate() only inserts "[attachment]" when at least one attachment exists, so these sticker-only updates end up as empty messages. Use msg.Sticker.Emoji or a [sticker] placeholder on this path.
🧰 Tools
🪛 golangci-lint (2.12.2)
[error] 306-306: SA9003: empty branch
(staticcheck)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/inbox/channel/telegram/telegram.go` around lines 296 - 309, When
handling animated/video stickers in the msg.Sticker branch (inside the
IsAnimated || IsVideo check) you must not leave the path that has no Thumbnail
empty; update that branch (where Thumbnail == nil) to add a fallback so
ProcessWebhookUpdate doesn't persist an empty message — either create and append
a small text attachment/placeholder to attachments or set the message content to
msg.Sticker.Emoji (if non-empty) or a literal "[sticker]" placeholder so that
attachments or content exist for ProcessWebhookUpdate to persist; modify the
code around t.downloadFile/msg.Sticker and attachments to implement this
fallback.
| payload, _ := json.Marshal(map[string]interface{}{ | ||
| "chat_id": chatID, | ||
| "text": text, | ||
| "parse_mode": "HTML", | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's view the code around lines 486-490
cat -n internal/inbox/channel/telegram/telegram.go | sed -n '475,505p'Repository: abhinavxd/libredesk
Length of output: 1152
🏁 Script executed:
# Let's also check the function signature and see where this text comes from
cat -n internal/inbox/channel/telegram/telegram.go | head -510 | tail -100Repository: abhinavxd/libredesk
Length of output: 3624
🏁 Script executed:
# Check if there are any HTML escaping utilities in the codebase
rg -i "html|escape" --type go internal/inbox/channel/telegram/Repository: abhinavxd/libredesk
Length of output: 131
🏁 Script executed:
# Search for all uses of parse_mode in the codebase
rg "parse_mode" --type goRepository: abhinavxd/libredesk
Length of output: 131
🏁 Script executed:
# Find all callers of sendMessage function
rg "\.sendMessage\(" --type go -B 2 -A 2Repository: abhinavxd/libredesk
Length of output: 423
🏁 Script executed:
# Check the broader context of how messages are sent - look for any processing
rg "sendMessage|parse_mode" --type go -B 5 -A 2Repository: abhinavxd/libredesk
Length of output: 1984
🌐 Web query:
Telegram Bot API HTML parse_mode special characters escaping requirements
💡 Result:
When using the HTML parse_mode in the Telegram Bot API, you must escape specific characters to ensure the message is parsed correctly and to prevent the API from misinterpreting your text as HTML tags or entities [1][2]. The required escaping rules for HTML parse_mode are: 1. Character Escaping: Any '<', '>', and '&' symbols that are not part of a valid HTML tag or entity must be replaced with their corresponding HTML entities [1][2]: - Replace '<' with '<' [1][2] - Replace '>' with '>' [1][2] - Replace '&' with '&' [1][2] 2. Supported Entities: The Telegram Bot API supports all numerical HTML entities and a limited set of named HTML entities: '<', '>', '&', and '"' [1][2]. 3. General Best Practices: - Do not nest tags [1][2]. - Use only the officially supported HTML tags (e.g., , , , , , ,
) [1][2][3]. - If you need to include line breaks, use the newline character '\n' instead of HTML tags like
[4][3]. Failure to escape these characters can result in API errors or malformed message formatting [4][3]. Many popular Telegram bot libraries (such as pyTelegramBotAPI or Telegraf) provide built-in utility functions to handle this escaping automatically, which is recommended to avoid manual errors [5][6][7].Citations:
- 1: https://stackoverflow.com/questions/49706034/using-html-in-telegram-bot
- 2: https://grammy.dev/ref/types/ParseMode
- 3: https://www.misterchatter.com/docs/telegram-html-formatting-guide-supported-tags/
- 4: https://stackoverflow.com/questions/57394179/what-html-tags-can-be-used-to-send-a-message-on-telegram-bot
- 5: What are all the "special characters" that need to be escaped, and more importantly, why? telegraf/telegraf#1242
- 6: https://pytba.readthedocs.io/en/latest/formatting.html
- 7: https://github.com/eternnoir/pyTelegramBotAPI/blob/master/examples/formatting_example.py
Remove parse_mode to send messages as plain text, or escape HTML entities.
The text parameter is passed directly without HTML escaping while parse_mode is set to "HTML". According to Telegram Bot API, this will cause parse-entity errors if the message contains <, >, &, or other unsupported sequences. Since messages appear to be raw agent text rather than intentional HTML, either remove parse_mode to send as plain text or implement HTML entity escaping for &, <, and >.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/inbox/channel/telegram/telegram.go` around lines 486 - 490, The
payload currently sets "parse_mode":"HTML" while passing raw text (see the
payload creation around variables payload, chatID, text), which will cause
Telegram parse errors for characters like &, <, >; either remove the
"parse_mode" key so messages are sent as plain text or HTML-escape the text
before marshaling. To fix: update the payload construction in telegram.go to
omit "parse_mode" when sending agent text, or run the text through an
HTML-escape helper (escape &, <, > to &, <, >) and then keep
"parse_mode":"HTML"; ensure you update where payload is json.Marshal'ed so the
chat_id/text pair uses the chosen approach consistently.
|
Hey I am gearing up for the next release 2.3.0 this weekend which adds support for inline images, bulk actions and bunch for performance improvements to reduce CPU and ram usage. I will check this after that. Thanks! |
|
Hey, I will check this over this weekend, telegram was much needed. Thank you. |
|
Hi,
Then it's a easy lookup, I am adding whatsapp channel next so I will be using this table for it.
|
|
The .gitignore file is corrupted |
| } | ||
|
|
||
| if err := tgInbox.ProcessWebhookUpdate(update); err != nil { | ||
| app.lo.Error("telegram webhook: error processing update", "error", err) |
There was a problem hiding this comment.
Should return error? instead of 200?
| } | ||
| // Upload avatar if provided as bytes. | ||
| if len(in.Contact.AvatarContent) > 0 { | ||
| avatarPath, err := m.uploadContactAvatar(in.Contact.AvatarContent, in.Contact.AvatarMIME) |
There was a problem hiding this comment.
It will upload avatar to store on every single message.
A better fix is
ProcessIncomingMessage, for channel == telegram: look up get-contact-id-by-channel-identity('telegram', <user_id>). (Here we query to the new table that I suggested in my other comment)
If Hit -> reuse that contact_id, skip avatar.
Miss -> create the contact (real name, no fake email)
| payload, _ := json.Marshal(map[string]interface{}{ | ||
| "chat_id": chatID, | ||
| "text": text, | ||
| "parse_mode": "HTML", |
There was a problem hiding this comment.
We are parsing this mode but sending only text_content here, so i think we can drop this.
Also the telegram HTML support looks limited - https://core.telegram.org/bots/api#formatting-options
| return secretToken == t.WebhookSecret() | ||
| } | ||
|
|
||
| func (t *Telegram) SetWebhook(url string) error { |
Summary
Adds Telegram Bot API as a new inbox channel, enabling agents to receive and reply to messages from Telegram users directly within LibreDesk.
Changes
Backend
internal/inbox/channel/telegram/package implementing theinbox.InboxinterfacePOST /api/v1/inboxes/telegram/{id}/webhook) for receiving Telegram updatesexternal_user_idfor Telegram user identificationtelegram_chat_idstored in message meta for reply routingtelegramto thechannelsenumFrontend
TelegramInboxForm.vuecomponent for create/edit with bot token fieldt.melinktelegram_prefix from external ID displayHow it works
Testing
Summary by CodeRabbit
New Features
Chores
Localization