AntFleet

Disagreement · eb14fff2-openai-1

Telegram getUpdates acknowledges all updates (including other chats), risking message loss outside target chat

mismatch
repo 6f7fc663·PR #2·reviewed 1 week ago

Primary finding

Telegram getUpdates acknowledges all updates (including other chats), risking message loss outside target chat

mediumdata-losshigh
  • .github/workflows/messages.yml:65-75
MAX_UPDATE_ID is updated for every update, not just those from the configured chat. Advancing the offset acknowledges and discards all pending updates globally for the bot, potentially dropping messages from other chats that this workflow does not process.

Recommendation

Either dedicate the bot token to a single chat, or process/forward all chats. If multiple chats must be preserved, consider maintaining a separate consumer that handles all updates or redesign to not advance offset beyond the highest update actually processed (note: Telegram uses a global offset; selective ack is not supported, so multi-chat bots require processing for all chats to avoid loss). Document this constraint explicitly if keeping current behavior.

Counterpart finding

Telegram offset acknowledged even when the chat id does not match — silently drops other-chat updates

lowdata-losshigh
  • .github/workflows/messages.yml:71-87
The `MAX_UPDATE_ID` is updated unconditionally on every iteration, then the loop calls `getUpdates?offset=$((MAX_UPDATE_ID+1))` to acknowledge. This silently discards updates from other Telegram chats the bot is in. That may be acceptable here (single-chat bot), but it's an undocumented behavior and means any debugging / multi-chat extension will lose messages without notice.

Recommendation

Either document that the bot expects to be in exactly one chat, or only advance the offset for explicitly-handled-or-ignored updates and emit a warning for unknown chats.

Why this didn't post

This finding didn't meet AntFleet's unanimous agreement threshold. Both frontier models review every PR independently; only findings they both flag with the same severity and category are posted to the PR. This one fell through.

read the methodology →