From 897f0122f470d7c59424a84f49c9570fc7303385 Mon Sep 17 00:00:00 2001 From: Lucas Rayan Date: Thu, 22 Oct 2026 22:05:43 -0300 Subject: [PATCH] fix(webmail): Undo send really undoes, and a send is not made twice The countdown sent from inside a state updater and Undo's button submitted the form, so Undo stopped the timer and then started the send over. The timer is now held in a ref, Undo is type=button, and leaving the screen flushes a pending send once. Snippets drop a stylesheet the preview was cut inside of. Co-Authored-By: Claude Sonnet 5.5 --- .wolf/buglog.json | 11 ++++ services/webmail/src/components/Composer.tsx | 60 +++++++++++++++---- .../src/compose-draft-integrity.test.ts | 29 +++++++++ services/webmail/src/lib/snippet.ts | 5 +- services/webmail/src/snippet.test.ts | 4 ++ 5 files changed, 97 insertions(+), 12 deletions(-) diff --git a/.wolf/buglog.json b/.wolf/buglog.json index 654c0f1..039f99d 100644 --- a/.wolf/buglog.json +++ b/.wolf/buglog.json @@ -35,6 +35,17 @@ "drafts" ], "date": "2026-10-22" + }, + { + "error_message": "Undo send did not undo: the message was sent anyway (and twice in development)", + "root_cause": "The 10-second countdown sent the message from inside a setUndoSeconds updater. Undo only set the number to null; the interval kept running, its next tick saw prev === null and sent. React may also run an updater twice", + "fix": "The countdown is counted in a plain closure with the timer kept in pendingSend; handleUndo clears it; leaving the screen flushes a pending send once (and does not navigate). Covered by compose-draft-integrity.test.ts", + "tags": [ + "webmail", + "compose", + "undo-send" + ], + "date": "2026-10-22" } ] } \ No newline at end of file diff --git a/services/webmail/src/components/Composer.tsx b/services/webmail/src/components/Composer.tsx index 00bfd81..5f5c8a7 100644 --- a/services/webmail/src/components/Composer.tsx +++ b/services/webmail/src/components/Composer.tsx @@ -87,6 +87,11 @@ export function Composer({ initial, variant = "page", onSent, onDiscarded }: Com // way when that happens would otherwise land afterwards and bring the draft // back, so every save checks this before and after it talks to the server. const discardedRef = useRef(false); + // The send waiting out its ten seconds, so Undo can stop it and leaving the + // screen cannot lose it. + const pendingSend = useRef<{ timer: ReturnType; html: string } | null>(null); + const sendRef = useRef<(html: string) => Promise>(async () => undefined); + const unmounted = useRef(false); const editorRef = useRef(null); // The body, mirrored into React state. @@ -98,6 +103,21 @@ export function Composer({ initial, variant = "page", onSent, onDiscarded }: Com // that is why a draft left behind by a sent message was empty. const [body, setBody] = useState(""); + // Leaving with a send still counting down sends it now. Dropping it would lose + // a message the writer was told was on its way; Undo is the way to cancel. + useEffect(() => { + unmounted.current = false; + return () => { + unmounted.current = true; + const pending = pendingSend.current; + if (pending) { + clearInterval(pending.timer); + pendingSend.current = null; + void sendRef.current(pending.html); + } + }; + }, []); + // An inline reply is opened to be written in, so the cursor starts in it. useEffect(() => { if (inline) editorRef.current?.focus(); @@ -290,16 +310,24 @@ export function Composer({ initial, variant = "page", onSent, onDiscarded }: Com const bodyHtml = editorRef.current?.innerHTML || body; - const interval = setInterval(() => { - setUndoSeconds((prev) => { - if (prev === null || prev <= 1) { - clearInterval(interval); - void sendActualMessage(bodyHtml); - return null; - } - return prev - 1; - }); + // The countdown is counted here and not inside a state updater. The send + // used to be a side effect of that updater, which React may run twice (the + // message went out twice) and which Undo could not stop: Undo only cleared + // the number, and the next tick - finding it empty - sent the message + // anyway. + let remaining = 10; + const timer = setInterval(() => { + remaining -= 1; + if (remaining > 0) { + setUndoSeconds(remaining); + return; + } + clearInterval(timer); + pendingSend.current = null; + setUndoSeconds(null); + void sendRef.current(bodyHtml); }, 1000); + pendingSend.current = { timer, html: bodyHtml }; }; const sendActualMessage = async (htmlBody: string) => { @@ -329,14 +357,22 @@ export function Composer({ initial, variant = "page", onSent, onDiscarded }: Com setSending(false); return; } - onSent(); + // A flush after leaving must not drag the writer back to Sent. + if (!unmounted.current) onSent(); } catch { setSendError(t("errors.serverError")); setSending(false); } }; + sendRef.current = sendActualMessage; + const handleUndo = () => { + // Stops the timer itself: clearing the number on screen is not enough. + if (pendingSend.current) { + clearInterval(pendingSend.current.timer); + pendingSend.current = null; + } setUndoSeconds(null); setSending(false); }; @@ -412,7 +448,9 @@ export function Composer({ initial, variant = "page", onSent, onDiscarded }: Com remaining) (9s remaining)". */} {t("mail.undoSend", { seconds: undoSeconds })} - diff --git a/services/webmail/src/compose-draft-integrity.test.ts b/services/webmail/src/compose-draft-integrity.test.ts index d89619f..0232f4b 100644 --- a/services/webmail/src/compose-draft-integrity.test.ts +++ b/services/webmail/src/compose-draft-integrity.test.ts @@ -119,3 +119,32 @@ describe("discarding a draft", () => { expect(compose).toMatch(/discardedRef\.current && draftUidRef\.current > 0/); }); }); + +describe("undoing a send", () => { + // The countdown sent the message from inside a state updater. Undo cleared the + // number on screen but not the timer, whose next tick found it empty and sent + // the message anyway - so "Undo" undid nothing. React may also run an updater + // twice, which sent it twice. + it("does not send from inside a state updater", () => { + const compose = code(COMPOSE); + expect(compose).not.toMatch(/setUndoSeconds\(\(prev\)/); + }); + + it("stops the timer when the writer undoes", () => { + const compose = code(COMPOSE); + expect(compose).toMatch(/const handleUndo = \(\) => \{[\s\S]*?clearInterval\(pendingSend\.current\.timer\)/); + }); + + // A button inside a form submits it unless it says otherwise. Undo stopped the + // timer and then submitted the form, which started the send over. + it("is a button that does not submit the form", () => { + const compose = code(COMPOSE); + expect(compose).toMatch(/