From da4fec664121b8ca443a3d72d19a6a1c9200204f Mon Sep 17 00:00:00 2001 From: Wayne <105773686+hougangdev@users.noreply.github.com> Date: Tue, 10 Mar 2026 12:47:39 +0800 Subject: [PATCH] fix(telegram): prevent duplicate messages when preview edit times out (#41662) Merged via squash. Prepared head SHA: 2780e62d070d7b4c4d7447e966ca172e33e44ad4 Co-authored-by: hougangdev <105773686+hougangdev@users.noreply.github.com> Co-authored-by: obviyus <22031114+obviyus@users.noreply.github.com> Reviewed-by: @obviyus --- CHANGELOG.md | 1 + src/telegram/bot-message-dispatch.test.ts | 204 +++++++++++++++++++ src/telegram/bot-message-dispatch.ts | 34 +++- src/telegram/lane-delivery-text-deliverer.ts | 158 ++++++++++---- src/telegram/lane-delivery.test.ts | 147 ++++++++++++- src/telegram/lane-delivery.ts | 1 + 6 files changed, 492 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f017b834209..e80e2c34ce4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ Docs: https://docs.openclaw.ai - Tools/web search: treat Brave `llm-context` grounding snippets as plain strings so `web_search` no longer returns empty snippet arrays in LLM Context mode. (#41387) thanks @zheliu2. - Telegram/exec approvals: reject `/approve` commands aimed at other bots, keep deterministic approval prompts visible when tool-result delivery fails, and stop resolved exact IDs from matching other pending approvals by prefix. (#37233) Thanks @huntharo. - Control UI/Sessions: restore single-column session table collapse on narrow viewport or container widths by moving the responsive table override next to the base grid rule and enabling inline-size container queries. (#12175) Thanks @benjipeng. +- Telegram/final preview delivery: split active preview lifecycle from cleanup retention so missing archived preview edits avoid duplicate fallback sends without clearing the live preview or blocking later in-place finalization. (#41662) thanks @hougangdev. ## 2026.3.8 diff --git a/src/telegram/bot-message-dispatch.test.ts b/src/telegram/bot-message-dispatch.test.ts index 7caa7cc3af7..4f5e2484d50 100644 --- a/src/telegram/bot-message-dispatch.test.ts +++ b/src/telegram/bot-message-dispatch.test.ts @@ -906,6 +906,131 @@ describe("dispatchTelegramMessage draft streaming", () => { expect(deliverReplies).not.toHaveBeenCalled(); }); + it("keeps the active preview when an archived final edit target is missing", async () => { + let answerMessageId: number | undefined; + let answerDraftParams: + | { + onSupersededPreview?: (preview: { messageId: number; textSnapshot: string }) => void; + } + | undefined; + const answerDraftStream = { + update: vi.fn().mockImplementation((text: string) => { + if (text.includes("Message B")) { + answerMessageId = 1002; + } + }), + flush: vi.fn().mockResolvedValue(undefined), + messageId: vi.fn().mockImplementation(() => answerMessageId), + clear: vi.fn().mockResolvedValue(undefined), + stop: vi.fn().mockResolvedValue(undefined), + forceNewMessage: vi.fn().mockImplementation(() => { + answerMessageId = undefined; + }), + }; + const reasoningDraftStream = createDraftStream(); + createTelegramDraftStream + .mockImplementationOnce((params) => { + answerDraftParams = params as typeof answerDraftParams; + return answerDraftStream; + }) + .mockImplementationOnce(() => reasoningDraftStream); + dispatchReplyWithBufferedBlockDispatcher.mockImplementation( + async ({ dispatcherOptions, replyOptions }) => { + await replyOptions?.onPartialReply?.({ text: "Message A partial" }); + await replyOptions?.onAssistantMessageStart?.(); + await replyOptions?.onPartialReply?.({ text: "Message B partial" }); + answerDraftParams?.onSupersededPreview?.({ + messageId: 1001, + textSnapshot: "Message A partial", + }); + + await dispatcherOptions.deliver({ text: "Message A final" }, { kind: "final" }); + return { queuedFinal: true }; + }, + ); + deliverReplies.mockResolvedValue({ delivered: true }); + editMessageTelegram.mockRejectedValue(new Error("400: Bad Request: message to edit not found")); + + await dispatchWithContext({ context: createContext(), streamMode: "partial" }); + + expect(editMessageTelegram).toHaveBeenCalledWith( + 123, + 1001, + "Message A final", + expect.any(Object), + ); + expect(answerDraftStream.clear).not.toHaveBeenCalled(); + expect(deliverReplies).not.toHaveBeenCalled(); + }); + + it("still finalizes the active preview after an archived final edit is retained", async () => { + let answerMessageId: number | undefined; + let answerDraftParams: + | { + onSupersededPreview?: (preview: { messageId: number; textSnapshot: string }) => void; + } + | undefined; + const answerDraftStream = { + update: vi.fn().mockImplementation((text: string) => { + if (text.includes("Message B")) { + answerMessageId = 1002; + } + }), + flush: vi.fn().mockResolvedValue(undefined), + messageId: vi.fn().mockImplementation(() => answerMessageId), + clear: vi.fn().mockResolvedValue(undefined), + stop: vi.fn().mockResolvedValue(undefined), + forceNewMessage: vi.fn().mockImplementation(() => { + answerMessageId = undefined; + }), + }; + const reasoningDraftStream = createDraftStream(); + createTelegramDraftStream + .mockImplementationOnce((params) => { + answerDraftParams = params as typeof answerDraftParams; + return answerDraftStream; + }) + .mockImplementationOnce(() => reasoningDraftStream); + dispatchReplyWithBufferedBlockDispatcher.mockImplementation( + async ({ dispatcherOptions, replyOptions }) => { + await replyOptions?.onPartialReply?.({ text: "Message A partial" }); + await replyOptions?.onAssistantMessageStart?.(); + await replyOptions?.onPartialReply?.({ text: "Message B partial" }); + answerDraftParams?.onSupersededPreview?.({ + messageId: 1001, + textSnapshot: "Message A partial", + }); + + await dispatcherOptions.deliver({ text: "Message A final" }, { kind: "final" }); + await dispatcherOptions.deliver({ text: "Message B final" }, { kind: "final" }); + return { queuedFinal: true }; + }, + ); + deliverReplies.mockResolvedValue({ delivered: true }); + editMessageTelegram + .mockRejectedValueOnce(new Error("400: Bad Request: message to edit not found")) + .mockResolvedValueOnce({ ok: true, chatId: "123", messageId: "1002" }); + + await dispatchWithContext({ context: createContext(), streamMode: "partial" }); + + expect(editMessageTelegram).toHaveBeenNthCalledWith( + 1, + 123, + 1001, + "Message A final", + expect.any(Object), + ); + expect(editMessageTelegram).toHaveBeenNthCalledWith( + 2, + 123, + 1002, + "Message B final", + expect.any(Object), + ); + expect(answerDraftStream.clear).not.toHaveBeenCalled(); + expect(deliverReplies).not.toHaveBeenCalled(); + }); + it.each(["partial", "block"] as const)( "keeps finalized text preview when the next assistant message is media-only (%s mode)", async (streamMode) => { @@ -1903,4 +2028,83 @@ describe("dispatchTelegramMessage draft streaming", () => { expect(draftA.clear).toHaveBeenCalledTimes(1); expect(draftB.clear).toHaveBeenCalledTimes(1); }); + + it("swallows post-connect network timeout on preview edit to prevent duplicate messages", async () => { + const draftStream = createDraftStream(999); + createTelegramDraftStream.mockReturnValue(draftStream); + dispatchReplyWithBufferedBlockDispatcher.mockImplementation( + async ({ dispatcherOptions, replyOptions }) => { + await replyOptions?.onPartialReply?.({ text: "Streaming..." }); + await dispatcherOptions.deliver({ text: "Final answer" }, { kind: "final" }); + return { queuedFinal: true }; + }, + ); + deliverReplies.mockResolvedValue({ delivered: true }); + // Simulate a post-connect timeout: editMessageTelegram throws a network + // error even though Telegram's server already processed the edit. + editMessageTelegram.mockRejectedValue(new Error("timeout: request timed out after 30000ms")); + + await dispatchWithContext({ context: createContext() }); + + expect(editMessageTelegram).toHaveBeenCalledTimes(1); + const deliverCalls = deliverReplies.mock.calls; + const finalTextSentViaDeliverReplies = deliverCalls.some((call: unknown[]) => + (call[0] as { replies?: Array<{ text?: string }> })?.replies?.some( + (r: { text?: string }) => r.text === "Final answer", + ), + ); + expect(finalTextSentViaDeliverReplies).toBe(false); + }); + + it("falls back to sendPayload on pre-connect error during final edit", async () => { + const draftStream = createDraftStream(999); + createTelegramDraftStream.mockReturnValue(draftStream); + dispatchReplyWithBufferedBlockDispatcher.mockImplementation( + async ({ dispatcherOptions, replyOptions }) => { + await replyOptions?.onPartialReply?.({ text: "Streaming..." }); + await dispatcherOptions.deliver({ text: "Final answer" }, { kind: "final" }); + return { queuedFinal: true }; + }, + ); + deliverReplies.mockResolvedValue({ delivered: true }); + const preConnectErr = new Error("connect ECONNREFUSED 149.154.167.220:443"); + (preConnectErr as NodeJS.ErrnoException).code = "ECONNREFUSED"; + editMessageTelegram.mockRejectedValue(preConnectErr); + + await dispatchWithContext({ context: createContext() }); + + expect(editMessageTelegram).toHaveBeenCalledTimes(1); + const deliverCalls = deliverReplies.mock.calls; + const finalTextSentViaDeliverReplies = deliverCalls.some((call: unknown[]) => + (call[0] as { replies?: Array<{ text?: string }> })?.replies?.some( + (r: { text?: string }) => r.text === "Final answer", + ), + ); + expect(finalTextSentViaDeliverReplies).toBe(true); + }); + + it("falls back when Telegram reports the current final edit target missing", async () => { + const draftStream = createDraftStream(999); + createTelegramDraftStream.mockReturnValue(draftStream); + dispatchReplyWithBufferedBlockDispatcher.mockImplementation( + async ({ dispatcherOptions, replyOptions }) => { + await replyOptions?.onPartialReply?.({ text: "Streaming..." }); + await dispatcherOptions.deliver({ text: "Final answer" }, { kind: "final" }); + return { queuedFinal: true }; + }, + ); + deliverReplies.mockResolvedValue({ delivered: true }); + editMessageTelegram.mockRejectedValue(new Error("400: Bad Request: message to edit not found")); + + await dispatchWithContext({ context: createContext() }); + + expect(editMessageTelegram).toHaveBeenCalledTimes(1); + const deliverCalls = deliverReplies.mock.calls; + const finalTextSentViaDeliverReplies = deliverCalls.some((call: unknown[]) => + (call[0] as { replies?: Array<{ text?: string }> })?.replies?.some( + (r: { text?: string }) => r.text === "Final answer", + ), + ); + expect(finalTextSentViaDeliverReplies).toBe(true); + }); }); diff --git a/src/telegram/bot-message-dispatch.ts b/src/telegram/bot-message-dispatch.ts index fee56211ae5..4d8d2b678e8 100644 --- a/src/telegram/bot-message-dispatch.ts +++ b/src/telegram/bot-message-dispatch.ts @@ -38,6 +38,7 @@ import { createLaneTextDeliverer, type DraftLaneState, type LaneName, + type LanePreviewLifecycle, } from "./lane-delivery.js"; import { createTelegramReasoningStepState, @@ -239,7 +240,14 @@ export const dispatchTelegramMessage = async ({ answer: createDraftLane("answer", canStreamAnswerDraft), reasoning: createDraftLane("reasoning", canStreamReasoningDraft), }; - const finalizedPreviewByLane: Record = { + // Active preview lifecycle answers "can this current preview still be + // finalized?" Cleanup retention is separate so archived-preview decisions do + // not poison the active lane. + const activePreviewLifecycleByLane: Record = { + answer: "transient", + reasoning: "transient", + }; + const retainPreviewOnCleanupByLane: Record = { answer: false, reasoning: false, }; @@ -288,7 +296,10 @@ export const dispatchTelegramMessage = async ({ // so it remains visible across tool boundaries. const materializedId = await answerLane.stream?.materialize?.(); const previewMessageId = materializedId ?? answerLane.stream?.messageId(); - if (typeof previewMessageId === "number" && !finalizedPreviewByLane.answer) { + if ( + typeof previewMessageId === "number" && + activePreviewLifecycleByLane.answer === "transient" + ) { archivedAnswerPreviews.push({ messageId: previewMessageId, textSnapshot: answerLane.lastPartialText, @@ -301,7 +312,8 @@ export const dispatchTelegramMessage = async ({ resetDraftLaneState(answerLane); if (didForceNewMessage) { // New assistant message boundary: this lane now tracks a fresh preview lifecycle. - finalizedPreviewByLane.answer = false; + activePreviewLifecycleByLane.answer = "transient"; + retainPreviewOnCleanupByLane.answer = false; } return didForceNewMessage; }; @@ -331,7 +343,7 @@ export const dispatchTelegramMessage = async ({ const ingestDraftLaneSegments = async (text: string | undefined) => { const split = splitTextIntoLaneSegments(text); const hasAnswerSegment = split.segments.some((segment) => segment.lane === "answer"); - if (hasAnswerSegment && finalizedPreviewByLane.answer) { + if (hasAnswerSegment && activePreviewLifecycleByLane.answer !== "transient") { // Some providers can emit the first partial of a new assistant message before // onAssistantMessageStart() arrives. Rotate preemptively so we do not edit // the previously finalized preview message with the next message's text. @@ -469,7 +481,8 @@ export const dispatchTelegramMessage = async ({ const deliverLaneText = createLaneTextDeliverer({ lanes, archivedAnswerPreviews, - finalizedPreviewByLane, + activePreviewLifecycleByLane, + retainPreviewOnCleanupByLane, draftMaxChars, applyTextToPayload, sendPayload, @@ -596,7 +609,8 @@ export const dispatchTelegramMessage = async ({ } if (info.kind === "final") { if (reasoningLane.hasStreamedMessage) { - finalizedPreviewByLane.reasoning = true; + activePreviewLifecycleByLane.reasoning = "complete"; + retainPreviewOnCleanupByLane.reasoning = true; } reasoningStepState.resetForNextStep(); } @@ -674,7 +688,8 @@ export const dispatchTelegramMessage = async ({ reasoningStepState.resetForNextStep(); if (skipNextAnswerMessageStartRotation) { skipNextAnswerMessageStartRotation = false; - finalizedPreviewByLane.answer = false; + activePreviewLifecycleByLane.answer = "transient"; + retainPreviewOnCleanupByLane.answer = false; return; } await rotateAnswerLaneForNewAssistantMessage(); @@ -682,7 +697,8 @@ export const dispatchTelegramMessage = async ({ // Even when no forceNewMessage happened (e.g. prior answer had no // streamed partials), the next partial belongs to a fresh lifecycle // and must not trigger late pre-rotation mid-message. - finalizedPreviewByLane.answer = false; + activePreviewLifecycleByLane.answer = "transient"; + retainPreviewOnCleanupByLane.answer = false; }) : undefined, onReasoningEnd: reasoningLane.stream @@ -731,7 +747,7 @@ export const dispatchTelegramMessage = async ({ (p) => p.deleteIfUnused === false && p.messageId === activePreviewMessageId, ); const shouldClear = - !finalizedPreviewByLane[laneState.laneName] && !hasBoundaryFinalizedActivePreview; + !retainPreviewOnCleanupByLane[laneState.laneName] && !hasBoundaryFinalizedActivePreview; const existing = streamCleanupStates.get(stream); if (!existing) { streamCleanupStates.set(stream, { shouldClear }); diff --git a/src/telegram/lane-delivery-text-deliverer.ts b/src/telegram/lane-delivery-text-deliverer.ts index f244d086657..c8eb10a9bb1 100644 --- a/src/telegram/lane-delivery-text-deliverer.ts +++ b/src/telegram/lane-delivery-text-deliverer.ts @@ -1,22 +1,36 @@ import type { ReplyPayload } from "../auto-reply/types.js"; import type { TelegramInlineButtons } from "./button-types.js"; import type { TelegramDraftStream } from "./draft-stream.js"; +import { isRecoverableTelegramNetworkError, isSafeToRetrySendError } from "./network-errors.js"; const MESSAGE_NOT_MODIFIED_RE = /400:\s*Bad Request:\s*message is not modified|MESSAGE_NOT_MODIFIED/i; +const MESSAGE_NOT_FOUND_RE = + /400:\s*Bad Request:\s*message to edit not found|MESSAGE_ID_INVALID|message can't be edited/i; + +function extractErrorText(err: unknown): string { + return typeof err === "string" + ? err + : err instanceof Error + ? err.message + : typeof err === "object" && err && "description" in err + ? typeof err.description === "string" + ? err.description + : "" + : ""; +} function isMessageNotModifiedError(err: unknown): boolean { - const text = - typeof err === "string" - ? err - : err instanceof Error - ? err.message - : typeof err === "object" && err && "description" in err - ? typeof err.description === "string" - ? err.description - : "" - : ""; - return MESSAGE_NOT_MODIFIED_RE.test(text); + return MESSAGE_NOT_MODIFIED_RE.test(extractErrorText(err)); +} + +/** + * Returns true when Telegram rejects an edit because the target message can no + * longer be resolved or edited. The caller still needs preview context to + * decide whether to retain a different visible preview or fall back to send. + */ +function isMissingPreviewMessageError(err: unknown): boolean { + return MESSAGE_NOT_FOUND_RE.test(extractErrorText(err)); } export type LaneName = "answer" | "reasoning"; @@ -35,12 +49,20 @@ export type ArchivedPreview = { deleteIfUnused?: boolean; }; -export type LaneDeliveryResult = "preview-finalized" | "preview-updated" | "sent" | "skipped"; +export type LanePreviewLifecycle = "transient" | "complete"; + +export type LaneDeliveryResult = + | "preview-finalized" + | "preview-retained" + | "preview-updated" + | "sent" + | "skipped"; type CreateLaneTextDelivererParams = { lanes: Record; archivedAnswerPreviews: ArchivedPreview[]; - finalizedPreviewByLane: Record; + activePreviewLifecycleByLane: Record; + retainPreviewOnCleanupByLane: Record; draftMaxChars: number; applyTextToPayload: (payload: ReplyPayload, text: string) => ReplyPayload; sendPayload: (payload: ReplyPayload) => Promise; @@ -80,6 +102,8 @@ type TryUpdatePreviewParams = { previewTextSnapshot?: string; }; +type PreviewEditResult = "edited" | "retained" | "fallback"; + type ConsumeArchivedAnswerPreviewParams = { lane: DraftLaneState; text: string; @@ -139,6 +163,10 @@ function resolvePreviewTarget(params: ResolvePreviewTargetParams): PreviewTarget export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { const getLanePreviewText = (lane: DraftLaneState) => lane.lastPartialText; + const markActivePreviewComplete = (laneName: LaneName) => { + params.activePreviewLifecycleByLane[laneName] = "complete"; + params.retainPreviewOnCleanupByLane[laneName] = true; + }; const isDraftPreviewLane = (lane: DraftLaneState) => lane.stream?.previewMode?.() === "draft"; const canMaterializeDraftFinal = ( lane: DraftLaneState, @@ -184,8 +212,9 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { previewButtons?: TelegramInlineButtons; updateLaneSnapshot: boolean; lane: DraftLaneState; - treatEditFailureAsDelivered: boolean; - }): Promise => { + finalTextAlreadyLanded: boolean; + retainAlternatePreviewOnMissingTarget: boolean; + }): Promise => { try { await params.editPreview({ laneName: args.laneName, @@ -198,26 +227,58 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { args.lane.lastPartialText = args.text; } params.markDelivered(); - return true; + return "edited"; } catch (err) { if (isMessageNotModifiedError(err)) { params.log( `telegram: ${args.laneName} preview ${args.context} edit returned "message is not modified"; treating as delivered`, ); params.markDelivered(); - return true; + return "edited"; } - if (args.treatEditFailureAsDelivered) { + if (args.context === "final") { + if (args.finalTextAlreadyLanded) { + params.log( + `telegram: ${args.laneName} preview final edit failed after stop flush; keeping existing preview (${String(err)})`, + ); + params.markDelivered(); + return "retained"; + } + if (isSafeToRetrySendError(err)) { + params.log( + `telegram: ${args.laneName} preview final edit failed before reaching Telegram; falling back to standard send (${String(err)})`, + ); + return "fallback"; + } + if (isMissingPreviewMessageError(err)) { + if (args.retainAlternatePreviewOnMissingTarget) { + params.log( + `telegram: ${args.laneName} preview final edit target missing; keeping alternate preview without fallback (${String(err)})`, + ); + params.markDelivered(); + return "retained"; + } + params.log( + `telegram: ${args.laneName} preview final edit target missing with no alternate preview; falling back to standard send (${String(err)})`, + ); + return "fallback"; + } + if (isRecoverableTelegramNetworkError(err, { allowMessageMatch: true })) { + params.log( + `telegram: ${args.laneName} preview final edit may have landed despite network error; keeping existing preview (${String(err)})`, + ); + params.markDelivered(); + return "retained"; + } params.log( - `telegram: ${args.laneName} preview ${args.context} edit failed after stop-created flush; treating as delivered (${String(err)})`, + `telegram: ${args.laneName} preview final edit rejected by Telegram; falling back to standard send (${String(err)})`, ); - params.markDelivered(); - return true; + return "fallback"; } params.log( `telegram: ${args.laneName} preview ${args.context} edit failed; falling back to standard send (${String(err)})`, ); - return false; + return "fallback"; } }; @@ -232,8 +293,12 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { context, previewMessageId: previewMessageIdOverride, previewTextSnapshot, - }: TryUpdatePreviewParams): Promise => { - const editPreview = (messageId: number, treatEditFailureAsDelivered: boolean) => + }: TryUpdatePreviewParams): Promise => { + const editPreview = ( + messageId: number, + finalTextAlreadyLanded: boolean, + retainAlternatePreviewOnMissingTarget: boolean, + ) => tryEditPreviewMessage({ laneName, messageId, @@ -242,13 +307,15 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { previewButtons, updateLaneSnapshot, lane, - treatEditFailureAsDelivered, + finalTextAlreadyLanded, + retainAlternatePreviewOnMissingTarget, }); const finalizePreview = ( previewMessageId: number, - treatEditFailureAsDelivered: boolean, + finalTextAlreadyLanded: boolean, hadPreviewMessage: boolean, - ): boolean | Promise => { + retainAlternatePreviewOnMissingTarget = false, + ): PreviewEditResult | Promise => { const currentPreviewText = previewTextSnapshot ?? getLanePreviewText(lane); const shouldSkipRegressive = shouldSkipRegressivePreviewUpdate({ currentPreviewText, @@ -258,12 +325,16 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { }); if (shouldSkipRegressive) { params.markDelivered(); - return true; + return "edited"; } - return editPreview(previewMessageId, treatEditFailureAsDelivered); + return editPreview( + previewMessageId, + finalTextAlreadyLanded, + retainAlternatePreviewOnMissingTarget, + ); }; if (!lane.stream) { - return false; + return "fallback"; } const previewTargetBeforeStop = resolvePreviewTarget({ lane, @@ -282,7 +353,7 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { context, }); if (typeof previewTargetAfterStop.previewMessageId !== "number") { - return false; + return "fallback"; } return finalizePreview(previewTargetAfterStop.previewMessageId, true, false); } @@ -296,12 +367,15 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { context, }); if (typeof previewTargetAfterStop.previewMessageId !== "number") { - return false; + return "fallback"; } + const activePreviewMessageId = lane.stream?.messageId(); return finalizePreview( previewTargetAfterStop.previewMessageId, false, previewTargetAfterStop.hadPreviewMessage, + typeof activePreviewMessageId === "number" && + activePreviewMessageId !== previewTargetAfterStop.previewMessageId, ); }; @@ -328,9 +402,13 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { previewMessageId: archivedPreview.messageId, previewTextSnapshot: archivedPreview.textSnapshot, }); - if (finalized) { + if (finalized === "edited") { return "preview-finalized"; } + if (finalized === "retained") { + params.retainPreviewOnCleanupByLane.answer = true; + return "preview-retained"; + } } // Send the replacement message first, then clean up the old preview. // This avoids the visual "disappear then reappear" flash. @@ -375,7 +453,7 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { return archivedResult; } } - if (canEditViaPreview && !params.finalizedPreviewByLane[laneName]) { + if (canEditViaPreview && params.activePreviewLifecycleByLane[laneName] === "transient") { await params.flushDraftLane(lane); if (laneName === "answer") { const archivedResultAfterFlush = await consumeArchivedAnswerPreviewForFinal({ @@ -396,7 +474,7 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { text, }); if (materialized) { - params.finalizedPreviewByLane[laneName] = true; + markActivePreviewComplete(laneName); return "preview-finalized"; } } @@ -409,10 +487,14 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { skipRegressive: "existingOnly", context: "final", }); - if (finalized) { - params.finalizedPreviewByLane[laneName] = true; + if (finalized === "edited") { + markActivePreviewComplete(laneName); return "preview-finalized"; } + if (finalized === "retained") { + markActivePreviewComplete(laneName); + return "preview-retained"; + } } else if (!hasMedia && !payload.isError && text.length > params.draftMaxChars) { params.log( `telegram: preview final too long for edit (${text.length} > ${params.draftMaxChars}); falling back to standard send`, @@ -452,7 +534,7 @@ export function createLaneTextDeliverer(params: CreateLaneTextDelivererParams) { skipRegressive: "always", context: "update", }); - if (updated) { + if (updated === "edited") { return "preview-updated"; } } diff --git a/src/telegram/lane-delivery.test.ts b/src/telegram/lane-delivery.test.ts index 1cd1d36cf4c..a2dae1f05b9 100644 --- a/src/telegram/lane-delivery.test.ts +++ b/src/telegram/lane-delivery.test.ts @@ -42,7 +42,8 @@ function createHarness(params?: { const deletePreviewMessage = vi.fn().mockResolvedValue(undefined); const log = vi.fn(); const markDelivered = vi.fn(); - const finalizedPreviewByLane: Record = { answer: false, reasoning: false }; + const activePreviewLifecycleByLane = { answer: "transient", reasoning: "transient" } as const; + const retainPreviewOnCleanupByLane = { answer: false, reasoning: false } as const; const archivedAnswerPreviews: Array<{ messageId: number; textSnapshot: string; @@ -52,7 +53,8 @@ function createHarness(params?: { const deliverLaneText = createLaneTextDeliverer({ lanes, archivedAnswerPreviews, - finalizedPreviewByLane, + activePreviewLifecycleByLane: { ...activePreviewLifecycleByLane }, + retainPreviewOnCleanupByLane: { ...retainPreviewOnCleanupByLane }, draftMaxChars: params?.draftMaxChars ?? 4_096, applyTextToPayload: (payload: ReplyPayload, text: string) => ({ ...payload, text }), sendPayload, @@ -129,7 +131,7 @@ describe("createLaneTextDeliverer", () => { expect(harness.sendPayload).not.toHaveBeenCalled(); }); - it("treats stop-created preview edit failures as delivered", async () => { + it("keeps stop-created preview when follow-up final edit fails", async () => { const harness = createHarness({ answerMessageIdAfterStop: 777 }); harness.editPreview.mockRejectedValue(new Error("500: edit failed after stop flush")); @@ -140,10 +142,12 @@ describe("createLaneTextDeliverer", () => { infoKind: "final", }); - expect(result).toBe("preview-finalized"); + expect(result).toBe("preview-retained"); expect(harness.editPreview).toHaveBeenCalledTimes(1); expect(harness.sendPayload).not.toHaveBeenCalled(); - expect(harness.log).toHaveBeenCalledWith(expect.stringContaining("treating as delivered")); + expect(harness.log).toHaveBeenCalledWith( + expect.stringContaining("failed after stop flush; keeping existing preview"), + ); }); it("treats 'message is not modified' preview edit errors as delivered", async () => { @@ -170,7 +174,7 @@ describe("createLaneTextDeliverer", () => { ); }); - it("falls back to normal delivery when editing an existing preview fails", async () => { + it("falls back to sendPayload when an existing preview final edit is rejected", async () => { const harness = createHarness({ answerMessageId: 999 }); harness.editPreview.mockRejectedValue(new Error("500: preview edit failed")); @@ -186,6 +190,69 @@ describe("createLaneTextDeliverer", () => { expect(harness.sendPayload).toHaveBeenCalledWith( expect.objectContaining({ text: "Hello final" }), ); + expect(harness.log).toHaveBeenCalledWith( + expect.stringContaining("edit rejected by Telegram; falling back"), + ); + }); + + it("falls back when Telegram reports the current final edit target missing", async () => { + const harness = createHarness({ answerMessageId: 999 }); + harness.editPreview.mockRejectedValue(new Error("400: Bad Request: message to edit not found")); + + const result = await harness.deliverLaneText({ + laneName: "answer", + text: "Hello final", + payload: { text: "Hello final" }, + infoKind: "final", + }); + + expect(result).toBe("sent"); + expect(harness.editPreview).toHaveBeenCalledTimes(1); + expect(harness.sendPayload).toHaveBeenCalledWith( + expect.objectContaining({ text: "Hello final" }), + ); + expect(harness.log).toHaveBeenCalledWith( + expect.stringContaining("edit target missing with no alternate preview; falling back"), + ); + }); + + it("falls back to sendPayload when the final edit fails before reaching Telegram", async () => { + const harness = createHarness({ answerMessageId: 999 }); + const err = Object.assign(new Error("connect ECONNREFUSED"), { code: "ECONNREFUSED" }); + harness.editPreview.mockRejectedValue(err); + + const result = await harness.deliverLaneText({ + laneName: "answer", + text: "Hello final", + payload: { text: "Hello final" }, + infoKind: "final", + }); + + expect(result).toBe("sent"); + expect(harness.sendPayload).toHaveBeenCalledWith( + expect.objectContaining({ text: "Hello final" }), + ); + expect(harness.log).toHaveBeenCalledWith( + expect.stringContaining("failed before reaching Telegram; falling back"), + ); + }); + + it("keeps preview when the final edit times out after the request may have landed", async () => { + const harness = createHarness({ answerMessageId: 999 }); + harness.editPreview.mockRejectedValue(new Error("timeout: request timed out after 30000ms")); + + const result = await harness.deliverLaneText({ + laneName: "answer", + text: "Hello final", + payload: { text: "Hello final" }, + infoKind: "final", + }); + + expect(result).toBe("preview-retained"); + expect(harness.sendPayload).not.toHaveBeenCalled(); + expect(harness.log).toHaveBeenCalledWith( + expect.stringContaining("may have landed despite network error; keeping existing preview"), + ); }); it("falls back to normal delivery when stop-created preview has no message id", async () => { @@ -362,6 +429,74 @@ describe("createLaneTextDeliverer", () => { expect(harness.markDelivered).not.toHaveBeenCalled(); }); + // ── Duplicate message regression tests ────────────────────────────────── + // During final delivery, only ambiguous post-connect failures keep the + // preview. Definite non-delivery falls back to a real send. + + it("falls back on API error during final", async () => { + const harness = createHarness({ answerMessageId: 999 }); + harness.editPreview.mockRejectedValue(new Error("500: Internal Server Error")); + + const result = await harness.deliverLaneText({ + laneName: "answer", + text: "Hello final", + payload: { text: "Hello final" }, + infoKind: "final", + }); + + expect(result).toBe("sent"); + expect(harness.editPreview).toHaveBeenCalledTimes(1); + expect(harness.sendPayload).toHaveBeenCalledTimes(1); + }); + + it("falls back when an archived preview edit target is missing and no alternate preview exists", async () => { + const harness = createHarness(); + harness.archivedAnswerPreviews.push({ + messageId: 5555, + textSnapshot: "Partial streaming...", + deleteIfUnused: true, + }); + harness.editPreview.mockRejectedValue(new Error("400: Bad Request: message to edit not found")); + + const result = await harness.deliverLaneText({ + laneName: "answer", + text: "Complete final answer", + payload: { text: "Complete final answer" }, + infoKind: "final", + }); + + expect(harness.editPreview).toHaveBeenCalledTimes(1); + expect(harness.sendPayload).toHaveBeenCalledWith( + expect.objectContaining({ text: "Complete final answer" }), + ); + expect(result).toBe("sent"); + expect(harness.deletePreviewMessage).toHaveBeenCalledWith(5555); + }); + + it("keeps the active preview when an archived final edit target is missing", async () => { + const harness = createHarness({ answerMessageId: 999 }); + harness.archivedAnswerPreviews.push({ + messageId: 5555, + textSnapshot: "Partial streaming...", + deleteIfUnused: true, + }); + harness.editPreview.mockRejectedValue(new Error("400: Bad Request: message to edit not found")); + + const result = await harness.deliverLaneText({ + laneName: "answer", + text: "Complete final answer", + payload: { text: "Complete final answer" }, + infoKind: "final", + }); + + expect(harness.editPreview).toHaveBeenCalledTimes(1); + expect(harness.sendPayload).not.toHaveBeenCalled(); + expect(result).toBe("preview-retained"); + expect(harness.log).toHaveBeenCalledWith( + expect.stringContaining("edit target missing; keeping alternate preview without fallback"), + ); + }); + it("deletes consumed boundary previews after fallback final send", async () => { const harness = createHarness(); harness.archivedAnswerPreviews.push({ diff --git a/src/telegram/lane-delivery.ts b/src/telegram/lane-delivery.ts index 213b05e1158..a9114b281ff 100644 --- a/src/telegram/lane-delivery.ts +++ b/src/telegram/lane-delivery.ts @@ -4,6 +4,7 @@ export { type DraftLaneState, type LaneDeliveryResult, type LaneName, + type LanePreviewLifecycle, } from "./lane-delivery-text-deliverer.js"; export { createLaneDeliveryStateTracker,