fix(telegram): honor removeAckAfterReply for status reactions#68067
fix(telegram): honor removeAckAfterReply for status reactions#68067steipete merged 6 commits intoopenclaw:mainfrom
Conversation
openclaw-barnacle
Bot
added
channel: telegram
Greptile SummaryThis PR fixes Telegram so
Confidence Score: 4/5Safe to merge after addressing the error-handling concern; the core fix is correct and tests are solid. The primary logic is correct and the new tests exercise both the enabled and disabled paths. The P2 concern (a extensions/telegram/src/bot-message-dispatch.ts — the Prompt To Fix All With AIThis is a comment left during a code review.
Path: extensions/telegram/src/bot-message-dispatch.ts
Line: 1005-1014
Comment:
**`setDone()` rejection swallows reaction cleanup**
If `setDone()` rejects (e.g. a transient Telegram API error), the single `.catch()` at the end absorbs it and the `reactionApi(chatId, msg.message_id, [])` call is never reached. The status reaction stays on the message indefinitely even though a final reply was delivered and `removeAckAfterReply` is `true`. The plain-ack path avoids this because `removeAckReactionAfterReply` runs independently of the ack result.
Consider scoping `.catch()` only to `setDone()` so the cleanup always runs when `removeAckAfterReply` is enabled:
```suggestion
if (statusReactionController) {
void statusReactionController
.setDone()
.catch((err: unknown) => {
logVerbose(`telegram: status reaction done failed: ${String(err)}`);
})
.then(async () => {
if (!removeAckAfterReply) return;
if (!msg.message_id || !reactionApi) return;
await sleepWithAbort(1500);
await reactionApi(chatId, msg.message_id, []);
})
.catch((err: unknown) => {
logVerbose(`telegram: status reaction finalize failed: ${String(err)}`);
});
} else {
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(telegram): honor removeAckAfterReply..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| void Promise.resolve(statusReactionController.setDone()) | ||
| .then(async () => { | ||
| if (!removeAckAfterReply) return; | ||
| if (!msg.message_id || !reactionApi) return; | ||
| await sleepWithAbort(1500); | ||
| await reactionApi(chatId, msg.message_id, []); | ||
| }) | ||
| .catch((err: unknown) => { | ||
| logVerbose(`telegram: status reaction finalize failed: ${String(err)}`); | ||
| }); |
There was a problem hiding this comment.
setDone() rejection swallows reaction cleanup
If setDone() rejects (e.g. a transient Telegram API error), the single .catch() at the end absorbs it and the reactionApi(chatId, msg.message_id, []) call is never reached. The status reaction stays on the message indefinitely even though a final reply was delivered and removeAckAfterReply is true. The plain-ack path avoids this because removeAckReactionAfterReply runs independently of the ack result.
Consider scoping .catch() only to setDone() so the cleanup always runs when removeAckAfterReply is enabled:
| void Promise.resolve(statusReactionController.setDone()) | |
| .then(async () => { | |
| if (!removeAckAfterReply) return; | |
| if (!msg.message_id || !reactionApi) return; | |
| await sleepWithAbort(1500); | |
| await reactionApi(chatId, msg.message_id, []); | |
| }) | |
| .catch((err: unknown) => { | |
| logVerbose(`telegram: status reaction finalize failed: ${String(err)}`); | |
| }); | |
| if (statusReactionController) { | |
| void statusReactionController | |
| .setDone() | |
| .catch((err: unknown) => { | |
| logVerbose(`telegram: status reaction done failed: ${String(err)}`); | |
| }) | |
| .then(async () => { | |
| if (!removeAckAfterReply) return; | |
| if (!msg.message_id || !reactionApi) return; | |
| await sleepWithAbort(1500); | |
| await reactionApi(chatId, msg.message_id, []); | |
| }) | |
| .catch((err: unknown) => { | |
| logVerbose(`telegram: status reaction finalize failed: ${String(err)}`); | |
| }); | |
| } else { |
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/telegram/src/bot-message-dispatch.ts
Line: 1005-1014
Comment:
**`setDone()` rejection swallows reaction cleanup**
If `setDone()` rejects (e.g. a transient Telegram API error), the single `.catch()` at the end absorbs it and the `reactionApi(chatId, msg.message_id, [])` call is never reached. The status reaction stays on the message indefinitely even though a final reply was delivered and `removeAckAfterReply` is `true`. The plain-ack path avoids this because `removeAckReactionAfterReply` runs independently of the ack result.
Consider scoping `.catch()` only to `setDone()` so the cleanup always runs when `removeAckAfterReply` is enabled:
```suggestion
if (statusReactionController) {
void statusReactionController
.setDone()
.catch((err: unknown) => {
logVerbose(`telegram: status reaction done failed: ${String(err)}`);
})
.then(async () => {
if (!removeAckAfterReply) return;
if (!msg.message_id || !reactionApi) return;
await sleepWithAbort(1500);
await reactionApi(chatId, msg.message_id, []);
})
.catch((err: unknown) => {
logVerbose(`telegram: status reaction finalize failed: ${String(err)}`);
});
} else {
```
How can I resolve this? If you propose a fix, please make it concise.
|
Addressed the Telegram error-path concern. What changed:
Validation:
|
|
Follow-up fix pushed for CI. Reason:
Fix:
Validation:
|
poiskgit
force-pushed
the
fix/telegram-remove-ack-after-reply
branch
from
5005f83 to
1cb846b
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1cb846bde0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| .then(async () => { | ||
| if (!removeAckAfterReply) {return;} | ||
| if (!msg.message_id || !reactionApi) {return;} | ||
| await sleepWithAbort(1500); |
There was a problem hiding this comment.
The new status-reaction cleanup path hardcodes 1500 ms before clearing reactions, so messages.statusReactions.timing.doneHoldMs overrides are ignored on Telegram when removeAckAfterReply is enabled. In deployments that customize status-reaction timing (for faster or longer visibility), Telegram now behaves differently from the configured contract and from other channels that honor doneHoldMs.
Useful? React with 👍 / 👎.
|
Follow-up fix pushed to fully align Telegram status-reaction finalization with the existing timing contract. What changed:
Validation:
|
steipete
force-pushed
the
fix/telegram-remove-ack-after-reply
branch
from
947fe51 to
52f98ec
Compare
2 participants
Summary
Fix Telegram so messages.removeAckAfterReply is honored when messages.statusReactions.enabled = true.
Problem
Telegram previously removed ack reactions only in the plain ack path.
When status reactions were enabled, the finalization flow stopped at setDone() and left the reaction on the message, even when
emoveAckAfterReply was set to rue.
Discord already handled this correctly, so behavior was inconsistent across channels.
Root cause
Telegram used a status reaction controller for the status-reaction path, but the finalize path never cleared the reaction after reply completion.
Change
In the Telegram finalize path:
emoveAckAfterReply is enabled, wait briefly
clear the reaction explicitly with Telegram's native reaction API:
eactionApi(chatId, msg.message_id, [])
emoveAckAfterReply cases for Telegram status reactions
Result
Telegram now behaves as expected:
emoveAckAfterReply = true
Validation