fix(telegram): isolate update offset state by bot token#22363
Closed
AIflow-Labs wants to merge 1 commit intoopenclaw:mainfrom
Closed
fix(telegram): isolate update offset state by bot token#22363AIflow-Labs wants to merge 1 commit intoopenclaw:mainfrom
AIflow-Labs wants to merge 1 commit intoopenclaw:mainfrom
Conversation
Comment on lines
+46
to
47
| if (!parsed || parsed.version !== STORE_VERSION) { | ||
| return null; |
Contributor
There was a problem hiding this comment.
version check rejects all v1 files regardless of token presence, contradicting the PR description's claim that "If token is omitted, behavior remains backward compatible". existing v1 offset files will be ignored even for legacy callers, potentially causing duplicate message processing on upgrade
Suggested change
| if (!parsed || parsed.version !== STORE_VERSION) { | |
| return null; | |
| if (!parsed || (parsed.version !== STORE_VERSION && parsed.version !== 1)) { | |
| return null; | |
| } | |
| // For V1 files, only use them if no token is expected | |
| if (parsed.version === 1) { | |
| return parsed; | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/telegram/update-offset-store.ts
Line: 46-47
Comment:
version check rejects all v1 files regardless of token presence, contradicting the PR description's claim that "If token is omitted, behavior remains backward compatible". existing v1 offset files will be ignored even for legacy callers, potentially causing duplicate message processing on upgrade
```suggestion
if (!parsed || (parsed.version !== STORE_VERSION && parsed.version !== 1)) {
return null;
}
// For V1 files, only use them if no token is expected
if (parsed.version === 1) {
return parsed;
}
```
How can I resolve this? If you propose a fix, please make it concise.
18 tasks
Member
|
you have been detected be spamming with unwarranted prs and issues and your issues and prs have been automatically closed. please read contributing guide Contributing.md. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Why this fixes #22300
The previous storage format only keyed offsets by account id, so multiple Telegram tokens/accounts could reuse stale updateId checkpoints and skip messages. Binding checkpoints to token fingerprint prevents cross-token interference while preserving existing file location and cleanup behavior.
Tests
Edge cases
Greptile Summary
This PR correctly solves cross-token update offset interference by introducing SHA-256 token fingerprinting in the Telegram polling state. The implementation migrates from V1 (account-scoped only) to V2 (account + token scoped) storage format.
Key changes:
update-offset-store.tsbumped to version 2, addedtokenFingerprintfield and validationmonitor.tsnow passes token to read/write operations (lines 124, 135)Migration behavior:
Users upgrading from V1 will have their existing offset files ignored, causing bots to start from fresh state. This is intentional to prevent cross-token contamination when V1 files lack fingerprints.
Test coverage:
Note on review comments:
Please disregard the inline comment on
update-offset-store.tsline 46. That comment incorrectly suggests allowing V1 files to be read without token validation, which would reintroduce the cross-token offset bug this PR fixes. The current implementation correctly rejects all V1 files to ensure migration safety.Confidence Score: 4/5
Last reviewed commit: de0df5b