diff --git a/extensions/telegram/src/bot-handlers.runtime.ts b/extensions/telegram/src/bot-handlers.runtime.ts index 8107f23119a..cf2e81f0174 100644 --- a/extensions/telegram/src/bot-handlers.runtime.ts +++ b/extensions/telegram/src/bot-handlers.runtime.ts @@ -1384,10 +1384,7 @@ export const registerTelegramHandlers = ({ logVerbose( `telegram: failed to resolve approval callback ${approvalCallback.approvalId}: ${errStr}`, ); - await replyToCallbackChat( - "❌ Failed to submit approval. Please try again or contact an admin.", - ); - return; + throw new TelegramRetryableCallbackError(resolveErr); } try { await clearCallbackButtons(); diff --git a/extensions/telegram/src/bot.create-telegram-bot.test.ts b/extensions/telegram/src/bot.create-telegram-bot.test.ts index 661e3988552..21edbd71536 100644 --- a/extensions/telegram/src/bot.create-telegram-bot.test.ts +++ b/extensions/telegram/src/bot.create-telegram-bot.test.ts @@ -26,6 +26,7 @@ const { middlewareUseSpy, onSpy, replySpy, + resolveExecApprovalSpy, sendAnimationSpy, sendChatActionSpy, sendMessageSpy, @@ -3106,6 +3107,72 @@ describe("createTelegramBot", () => { expect(sendMessageSpy.mock.calls[0]?.[1]).toContain("plugin bind approval"); }); + it("retries exec approval callbacks after a bubbled resolution failure", async () => { + loadConfig.mockReturnValue({ + channels: { + telegram: { + dmPolicy: "open", + allowFrom: ["*"], + execApprovals: { + enabled: true, + approvers: ["9"], + target: "dm", + }, + }, + }, + }); + createTelegramBot({ token: "tok" }); + const callbackHandler = getOnHandler("callback_query"); + const middlewares = middlewareUseSpy.mock.calls + .map((call) => call[0]) + .filter( + (fn): fn is (ctx: Record, next: () => Promise) => Promise => + typeof fn === "function", + ); + const runMiddlewareChain = async (ctx: Record) => { + let idx = -1; + const dispatch = async (i: number): Promise => { + if (i <= idx) { + throw new Error("middleware dispatch called multiple times"); + } + idx = i; + const fn = middlewares[i]; + if (!fn) { + await callbackHandler(ctx); + return; + } + await fn(ctx, async () => dispatch(i + 1)); + }; + await dispatch(0); + }; + + resolveExecApprovalSpy.mockRejectedValueOnce(new Error("approval boom")); + + const ctx = { + update: { update_id: 8895 }, + callbackQuery: { + id: "cbq-approval-retry-1", + data: "/approve 138e9b8c allow-once", + from: { id: 9, first_name: "Ada", username: "ada_bot" }, + message: { + chat: { id: 1234, type: "private" }, + date: 1736380800, + message_id: 231, + text: "Approval required.", + }, + }, + me: { username: "openclaw_bot" }, + getFile: async () => ({ download: async () => new Uint8Array() }), + }; + + await expect(runMiddlewareChain(ctx)).rejects.toThrow("approval boom"); + await runMiddlewareChain(ctx); + + expect(resolveExecApprovalSpy).toHaveBeenCalledTimes(2); + expect(editMessageReplyMarkupSpy).toHaveBeenCalledTimes(1); + expect(sendMessageSpy).not.toHaveBeenCalled(); + }); + it("retries model provider callbacks after a bubbled edit failure", async () => { createTelegramBot({ token: "tok" }); const callbackHandler = getOnHandler("callback_query"); diff --git a/extensions/telegram/src/bot.test.ts b/extensions/telegram/src/bot.test.ts index 9e36469a87e..abcf38d8ab4 100644 --- a/extensions/telegram/src/bot.test.ts +++ b/extensions/telegram/src/bot.test.ts @@ -41,7 +41,6 @@ const loadConfig = getLoadConfigMock(); const loadWebMedia = getLoadWebMediaMock(); const readChannelAllowFromStore = getReadChannelAllowFromStoreMock(); const PUZZLE_EMOJI = "\u{1F9E9}"; -const CROSS_MARK_EMOJI = "\u{274C}"; const INFO_EMOJI = "\u{2139}\u{FE0F}"; const CHECK_MARK_EMOJI = "\u{2705}"; const THUMBS_UP_EMOJI = "\u{1F44D}"; @@ -501,9 +500,10 @@ describe("createTelegramBot", () => { expect(answerCallbackQuerySpy).toHaveBeenCalledWith("cbq-approve-blocked"); }); - it("does not leak raw approval callback errors back into Telegram chat", async () => { + it("keeps approval callback resolution failures out of Telegram chat before retry", async () => { onSpy.mockClear(); sendMessageSpy.mockClear(); + editMessageReplyMarkupSpy.mockClear(); resolveExecApprovalSpy.mockClear(); resolveExecApprovalSpy.mockRejectedValueOnce(new Error("gateway secret detail")); @@ -525,26 +525,27 @@ describe("createTelegramBot", () => { ctx: Record, ) => Promise; - await callbackHandler({ - callbackQuery: { - id: "cbq-approve-error", - data: "/approve 138e9b8c allow-once", - from: { id: 9, first_name: "Ada", username: "ada_bot" }, - message: { - chat: { id: 1234, type: "private" }, - date: 1736380800, - message_id: 25, - text: "Approval required.", + await expect( + callbackHandler({ + callbackQuery: { + id: "cbq-approve-error", + data: "/approve 138e9b8c allow-once", + from: { id: 9, first_name: "Ada", username: "ada_bot" }, + message: { + chat: { id: 1234, type: "private" }, + date: 1736380800, + message_id: 25, + text: "Approval required.", + }, }, - }, - me: { username: "openclaw_bot" }, - getFile: async () => ({ download: async () => new Uint8Array() }), - }); + me: { username: "openclaw_bot" }, + getFile: async () => ({ download: async () => new Uint8Array() }), + }), + ).rejects.toThrow("gateway secret detail"); - expect(sendMessageSpy).toHaveBeenCalledTimes(1); - expect(sendMessageSpy.mock.calls[0]?.[1]).toBe( - `${CROSS_MARK_EMOJI} Failed to submit approval. Please try again or contact an admin.`, - ); + expect(sendMessageSpy).not.toHaveBeenCalled(); + expect(editMessageReplyMarkupSpy).not.toHaveBeenCalled(); + expect(answerCallbackQuerySpy).toHaveBeenCalledWith("cbq-approve-error"); }); it("allows exec approval callbacks from target-only Telegram recipients", async () => { @@ -608,12 +609,13 @@ describe("createTelegramBot", () => { expect(answerCallbackQuerySpy).toHaveBeenCalledWith("cbq-approve-target"); }); - it("does not allow target-only recipients to use legacy plugin fallback ids", async () => { + it("keeps legacy plugin fallback approval failures retryable for target-only recipients", async () => { onSpy.mockClear(); editMessageReplyMarkupSpy.mockClear(); editMessageTextSpy.mockClear(); resolveExecApprovalSpy.mockClear(); replySpy.mockClear(); + sendMessageSpy.mockClear(); resolveExecApprovalSpy.mockRejectedValueOnce(new Error("unknown or expired approval id")); loadConfig.mockReturnValue({ @@ -637,21 +639,23 @@ describe("createTelegramBot", () => { ) => Promise; expect(callbackHandler).toBeDefined(); - await callbackHandler({ - callbackQuery: { - id: "cbq-legacy-plugin-fallback-blocked", - data: "/approve 138e9b8c allow-once", - from: { id: 9, first_name: "Ada", username: "ada_bot" }, - message: { - chat: { id: 1234, type: "private" }, - date: 1736380800, - message_id: 25, - text: "Legacy plugin approval required.", + await expect( + callbackHandler({ + callbackQuery: { + id: "cbq-legacy-plugin-fallback-blocked", + data: "/approve 138e9b8c allow-once", + from: { id: 9, first_name: "Ada", username: "ada_bot" }, + message: { + chat: { id: 1234, type: "private" }, + date: 1736380800, + message_id: 25, + text: "Legacy plugin approval required.", + }, }, - }, - me: { username: "openclaw_bot" }, - getFile: async () => ({ download: async () => new Uint8Array() }), - }); + me: { username: "openclaw_bot" }, + getFile: async () => ({ download: async () => new Uint8Array() }), + }), + ).rejects.toThrow("unknown or expired approval id"); expect(resolveExecApprovalSpy).toHaveBeenCalledWith({ cfg: expect.objectContaining({ @@ -669,11 +673,7 @@ describe("createTelegramBot", () => { }); expect(editMessageReplyMarkupSpy).not.toHaveBeenCalled(); expect(replySpy).not.toHaveBeenCalled(); - expect(sendMessageSpy).toHaveBeenCalledWith( - 1234, - `${CROSS_MARK_EMOJI} Failed to submit approval. Please try again or contact an admin.`, - undefined, - ); + expect(sendMessageSpy).not.toHaveBeenCalled(); expect(answerCallbackQuerySpy).toHaveBeenCalledWith("cbq-legacy-plugin-fallback-blocked"); });