From 82699f7a20c0ac94bf482b4cc271e2570f3f5349 Mon Sep 17 00:00:00 2001 From: claude_code Date: Fri, 26 Jun 2026 04:44:58 +0300 Subject: [PATCH] fix(ai-chat): guard "Send now" against a stale-status race (#198 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the open findings from the #198 ("Interrupt agent / send now") code review (posted on PR #200's thread). - sendNow: branch on the live status read from a new `statusRef` (updated every render) instead of the closure-captured `isStreaming`. If the turn finished between render and click, stale-true `isStreaming` made sendNow arm flushOnAbortRef/interruptNextSendRef and call a no-op stop(), leaving the one-shot flags armed to leak into a later turn. Reading the live status takes the not-streaming branch (send immediately) instead. Dependency array trimmed from [isStreaming, setQueue, stop] to [setQueue, stop]. - queued-state comment: drop the incorrect "(onFinish does not fire then)" claim — onFinish fires on every terminal outcome, only a clean finish flushes the queue, and a deliberate "Send now" flushes the promoted head via the abort branch of onFinish. The third review finding (missing i18n keys "Send now" / "Interrupt and send now") was already resolved in f789be9c. Co-Authored-By: Claude Opus 4.8 --- .../ai-chat/components/chat-thread.tsx | 29 +++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/apps/client/src/features/ai-chat/components/chat-thread.tsx b/apps/client/src/features/ai-chat/components/chat-thread.tsx index 024fd316..c54f83e4 100644 --- a/apps/client/src/features/ai-chat/components/chat-thread.tsx +++ b/apps/client/src/features/ai-chat/components/chat-thread.tsx @@ -182,9 +182,12 @@ export default function ChatThread({ // LOCAL state so it is scoped to this conversation: it is cleared when the user // deliberately switches chat / starts a new chat (the parent remounts this via // `key`), but it SURVIVES in-place new-chat id adoption (no remount), so a - // message queued during a brand-new chat's first turn is not lost. On Stop or - // error the queue is intentionally preserved (onFinish does not fire then) so - // the user decides what to do with the pending messages. + // message queued during a brand-new chat's first turn is not lost. On a normal + // Stop / disconnect / error the queue is intentionally preserved (onFinish still + // fires on every terminal outcome, but only a CLEAN finish flushes it) so the + // user decides what to do with the pending messages. The one exception is a + // deliberate "Send now", which also calls stop() and then flushes the promoted + // head via the abort branch of onFinish (see sendNow / onFinish below). const [queued, setQueued] = useState([]); // Mirror the queue in a ref so the `onFinish` flush always reads the latest // queue without a stale closure; `setQueue` updates BOTH the ref and the state. @@ -347,12 +350,28 @@ export default function ChatThread({ const isStreaming = status === "submitted" || status === "streaming"; + // Mirror the live `status` in a ref (updated every render) so event handlers + // fired from a stale render — notably `sendNow` clicked in the sub-frame window + // after a turn finished but before the re-render — branch on the CURRENT status, + // not the value captured in their closure. Prevents arming the interrupt/flush + // refs when nothing is actually streaming (see sendNow). + const statusRef = useRef(status); + statusRef.current = status; + // "Send now" on a queued message: interrupt the current turn and immediately // send THIS message. Any other queued messages stay queued and flush normally // after the new turn finishes. const sendNow = useCallback( (id: string) => { - if (isStreaming) { + // Branch on the LIVE status (statusRef), not the closure-captured + // `isStreaming`: if the turn finished between this render and the click, + // treat it as not-streaming and send immediately, instead of arming a + // flush/interrupt that no abort will ever consume (the stop() below would be + // a no-op and the flags would leak into a later turn). + const liveStatus = statusRef.current; + const liveStreaming = + liveStatus === "submitted" || liveStatus === "streaming"; + if (liveStreaming) { // Promote the chosen message to the head so the existing onFinish→flushNext // sends exactly it, then interrupt: the abort triggers onFinish below. setQueue(promoteToHead(queuedRef.current, id)); @@ -367,7 +386,7 @@ export default function ChatThread({ sendMessageRef.current?.({ text: msg.text }); } }, - [isStreaming, setQueue, stop], + [setQueue, stop], ); // Clear the stopped marker as soon as a new turn begins streaming, and drop any