fix(ai-chat): guard "Send now" against a stale-status race (#198 review)
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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<QueuedMessage[]>([]);
|
||||
// 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
|
||||
|
||||
Reference in New Issue
Block a user