fix: scope Telegram update offset to bot token#11347
fix: scope Telegram update offset to bot token#11347anooprdawar wants to merge 1 commit intoopenclaw:mainfrom
Conversation
src/telegram/update-offset-store.ts
Outdated
| if (parsed.botId && params.botToken) { | ||
| const currentBotId = extractBotIdFromToken(params.botToken); | ||
| if (currentBotId && parsed.botId !== currentBotId) { | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
Token change not invalidated
readTelegramUpdateOffset only discards a stored botId when extractBotIdFromToken(params.botToken) succeeds. If the new token is malformed/unset (but still passed in) and currentBotId is undefined, the stale offset will be accepted and you can still end up skipping all inbound messages for the new bot. Consider treating “token provided but botId can’t be extracted” as a mismatch (return null) when the file contains a botId.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/telegram/update-offset-store.ts
Line: 71:76
Comment:
**Token change not invalidated**
`readTelegramUpdateOffset` only discards a stored `botId` when `extractBotIdFromToken(params.botToken)` succeeds. If the new token is malformed/unset (but still passed in) and `currentBotId` is `undefined`, the stale offset will be accepted and you can still end up skipping all inbound messages for the new bot. Consider treating “token provided but botId can’t be extracted” as a mismatch (return `null`) when the file contains a `botId`.
How can I resolve this? If you propose a fix, please make it concise.bfc1ccb to
f92900f
Compare
|
This pull request has been automatically marked as stale due to inactivity. |
|
Useful direction here, and the scope is focused. Main still reads/writes offset files without token identity, so this fix family remains relevant. Before merge we should harden one edge case: when a token is provided but bot-id extraction fails, stale offsets should not be trusted. Please rebase and include that guard explicitly in tests; then we can review as a candidate that preserves older lineage while reducing token-rotation regressions. |
6beba80 to
582b33e
Compare
|
Thanks @vincentkoc — rebased onto main and hardened the malformed-token edge case. What changed:
All 13 tests passing. |
The update offset file was keyed only by account name, not by bot token. When a bot token changed (common during initial setup), the persisted lastUpdateId from the old token would silently reject all inbound messages because shouldSkipUpdate() saw updateId <= lastUpdateId. Changes: - Store botId (extracted from token) in the offset file - On read, validate botId matches current token; discard if mismatched - When botToken is provided but extraction fails (malformed token), discard the offset unconditionally — stale offsets must not be trusted - Pass botToken through read/write calls in monitor.ts - Backward compatible: legacy files without botId still work - Tests use withStateDirEnv helper, cover token change, malformed token (with and without stored botId), empty string, and legacy files Fixes openclaw#11337 Related: openclaw#11011
582b33e to
f9af41f
Compare
|
Superseded by #24549 (merged on 2026-02-23), which rebased and integrated this token-scoped Telegram offset fix with current Thank you @anooprdawar for the original implementation. |
|
Closing as superseded by #24549 (merged). Credit preserved in changelog. |
Problem
The update offset file (
~/.openclaw/telegram/update-offset-{account}.json) was keyed only by account name, not by bot token. When a bot token changed (common during initial setup or debugging), the persistedlastUpdateIdfrom the old token would silently reject all inbound messages from the new bot.shouldSkipUpdate()sawupdateId <= lastUpdateIdas always true because the old offset (e.g. 432M) was far ahead of the new bot's actual update IDs (e.g. 363M).Root Cause
Discovered while debugging #11011. The offset file had no awareness of which bot it belonged to. Token changes left a stale, impossibly-high offset that caused every inbound message to be dedupe-skipped.
Changes
update-offset-store.ts: StorebotId(extracted from token) in the offset JSON. On read, validatebotIdmatches the current token — discard offset if mismatched.monitor.ts: PassbotTokenthroughreadTelegramUpdateOffsetandwriteTelegramUpdateOffsetcalls.botIdfield still work (no forced migration).extractBotIdFromToken.Test Results
Fixes #11337
Related: #11011
Greptile Overview
Greptile Summary
This PR fixes a Telegram polling edge case where the persisted
lastUpdateIdoffset was keyed only by account id. It now stores the bot’s numeric id (derived from the bot token) alongside the offset, and on read discards the stored offset if the current token belongs to a different bot. The monitor passes the token through to the offset store, and new tests cover token-change invalidation, backward-compat reads, legacy files, and bot-id extraction.Confidence Score: 4/5
botTokenis provided but can’t be parsed into a bot id while the stored file has abotId(leading to the original skip-all-updates failure mode).(2/5) Greptile learns from your feedback when you react with thumbs up/down!