From 544a68239dc3c4eeb2fb071b8e3c900fedfc1262 Mon Sep 17 00:00:00 2001 From: moningbird <1553240477@qq.com> Date: Wed, 20 May 2026 10:42:20 +0800 Subject: [PATCH] Prevent duplicate chat sends while pending (#2071) Co-authored-by: Steven Enamakel --- app/src/pages/Conversations.tsx | 28 ++- .../__tests__/Conversations.render.test.tsx | 161 ++++++++++++++++++ 2 files changed, 185 insertions(+), 4 deletions(-) diff --git a/app/src/pages/Conversations.tsx b/app/src/pages/Conversations.tsx index 593ec063f..d0d9d7f51 100644 --- a/app/src/pages/Conversations.tsx +++ b/app/src/pages/Conversations.tsx @@ -250,6 +250,7 @@ const Conversations = ({ variant = 'page', composer = 'text' }: ConversationsPro const [inlineSuggestionValue, setInlineSuggestionValue] = useState(''); const [sendError, setSendError] = useState(null); const [sendAdvisory, setSendAdvisory] = useState(null); + const [pendingSendingThreadId, setPendingSendingThreadId] = useState(null); const [profileDraftOpen, setProfileDraftOpen] = useState(false); const [profileDraft, setProfileDraft] = useState(DEFAULT_PROFILE_DRAFT); const socketStatus = useAppSelector(selectSocketStatus); @@ -316,6 +317,7 @@ const Conversations = ({ variant = 'page', composer = 'text' }: ConversationsPro const textInputRef = useRef(null); const isComposingTextRef = useRef(false); + const pendingSendRef = useRef(null); const mediaRecorderRef = useRef(null); const mediaStreamRef = useRef(null); const audioChunksRef = useRef([]); @@ -545,6 +547,8 @@ const Conversations = ({ variant = 'page', composer = 'text' }: ConversationsPro dispatch(setActiveThread(null)); sendingTimeoutRef.current = null; sendingThreadIdRef.current = null; + pendingSendRef.current = null; + setPendingSendingThreadId(null); }, 120_000); }; @@ -668,6 +672,8 @@ const Conversations = ({ variant = 'page', composer = 'text' }: ConversationsPro }; const handleSendMessage = async (text?: string) => { + if (pendingSendRef.current) return; + const normalized = text ?? inputValue; const trimmedInput = normalized.trim(); @@ -707,6 +713,8 @@ const Conversations = ({ variant = 'page', composer = 'text' }: ConversationsPro const sendingThreadId = selectedThreadId; if (!sendingThreadId) return; + pendingSendRef.current = sendingThreadId; + setPendingSendingThreadId(sendingThreadId); const userMessage: ThreadMessage = { id: `msg_${globalThis.crypto.randomUUID()}`, content: trimmed, @@ -725,10 +733,14 @@ const Conversations = ({ variant = 'page', composer = 'text' }: ConversationsPro // unrelated errors whose `.toString()` happens to equal the sentinel. if (error === THREAD_NOT_FOUND_MESSAGE) { setSendError(null); + pendingSendRef.current = null; + setPendingSendingThreadId(null); return; } const msg = error instanceof Error ? error.message : String(error); setSendError(chatSendError('cloud_send_failed', msg)); + pendingSendRef.current = null; + setPendingSendingThreadId(null); return; } setInputValue(''); @@ -757,6 +769,11 @@ const Conversations = ({ variant = 'page', composer = 'text' }: ConversationsPro locale: uiLocale, }); trackEvent('chat_message_sent'); + // Backend accepted the send; lifecycle ('started' → 'streaming') now + // owns the `isSending` UI lock. Release the pending guard so the next + // user turn isn't blocked by a stale ref/state. + pendingSendRef.current = null; + setPendingSendingThreadId(null); // Active-thread reset happens in the global ChatRuntimeProvider events. } catch (err) { @@ -780,6 +797,8 @@ const Conversations = ({ variant = 'page', composer = 'text' }: ConversationsPro } dispatch(clearRuntimeForThread({ threadId: sendingThreadId })); dispatch(setActiveThread(null)); + pendingSendRef.current = null; + setPendingSendingThreadId(null); } }; @@ -1079,7 +1098,8 @@ const Conversations = ({ variant = 'page', composer = 'text' }: ConversationsPro }, [selectedThreadId, composerInteractionBlocked, inputMode]); const isSending = Boolean( selectedThreadId && - (inferenceTurnLifecycleByThread[selectedThreadId] === 'started' || + (pendingSendingThreadId === selectedThreadId || + inferenceTurnLifecycleByThread[selectedThreadId] === 'started' || inferenceTurnLifecycleByThread[selectedThreadId] === 'streaming') ); const shouldRenderTimelineBeforeLatestAgentMessage = @@ -1972,7 +1992,7 @@ const Conversations = ({ variant = 'page', composer = 'text' }: ConversationsPro // Without `!selectedThreadId`, a mic submit before a thread is // ready hits `handleSendMessage`'s early return and the // transcript is silently dropped — the user spoke into the void. - disabled={composerInteractionBlocked || !selectedThreadId} + disabled={composerInteractionBlocked || isSending || !selectedThreadId} onSubmit={text => handleSendMessage(text)} onError={message => setSendError(chatSendError('voice_transcription', message))} showDeviceSelector @@ -2001,7 +2021,7 @@ const Conversations = ({ variant = 'page', composer = 'text' }: ConversationsPro onKeyDown={handleInputKeyDown} placeholder={t('chat.typeMessage')} rows={1} - disabled={composerInteractionBlocked} + disabled={composerInteractionBlocked || isSending} className="relative z-10 w-full resize-none border-0 bg-transparent pl-4 pr-10 py-2.5 text-sm leading-normal whitespace-pre-wrap break-words font-sans text-stone-900 dark:text-neutral-100 placeholder:text-stone-400 dark:placeholder:text-neutral-500 outline-none focus:outline-none focus-visible:outline-none focus:ring-0 focus-visible:ring-0 max-h-32 disabled:opacity-50 disabled:cursor-not-allowed" /> {/* Voice input mic hidden per #717 (inputMode='voice' path retained). */} @@ -2012,7 +2032,7 @@ const Conversations = ({ variant = 'page', composer = 'text' }: ConversationsPro onClick={() => { void handleSendMessage(); }} - disabled={!inputValue.trim() || composerInteractionBlocked} + disabled={!inputValue.trim() || composerInteractionBlocked || isSending} className="w-10 h-10 flex items-center justify-center rounded-full bg-primary-500 hover:bg-primary-600 text-white disabled:opacity-40 disabled:cursor-not-allowed transition-colors flex-shrink-0"> {isSending ? ( diff --git a/app/src/pages/__tests__/Conversations.render.test.tsx b/app/src/pages/__tests__/Conversations.render.test.tsx index d2d06815b..dde95cfc7 100644 --- a/app/src/pages/__tests__/Conversations.render.test.tsx +++ b/app/src/pages/__tests__/Conversations.render.test.tsx @@ -16,6 +16,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'; import { agentProfilesApi } from '../../services/api/agentProfilesApi'; import { threadApi } from '../../services/api/threadApi'; import { chatSend } from '../../services/chatService'; +import { CoreRpcError } from '../../services/coreRpcClient'; import agentProfileReducer from '../../store/agentProfileSlice'; import chatRuntimeReducer from '../../store/chatRuntimeSlice'; import socketReducer from '../../store/socketSlice'; @@ -646,6 +647,166 @@ describe('Conversations — smoke render (#1123 welcome-lock removal)', () => { }); }); + it('blocks duplicate sends while the first send is still pending', async () => { + let resolveSend: (() => void) | undefined; + vi.mocked(chatSend).mockImplementationOnce( + () => + new Promise(resolve => { + resolveSend = resolve; + }) + ); + const { textarea, thread } = await renderSelectedConversation(); + + await act(async () => { + fireEvent.change(textarea, { target: { value: 'slow backend' } }); + }); + await waitFor(() => { + expect(textarea).toHaveValue('slow backend'); + expect(screen.getByRole('button', { name: 'Send message' })).not.toBeDisabled(); + }); + + const sendButton = screen.getByRole('button', { name: 'Send message' }); + await act(async () => { + fireEvent.click(sendButton); + fireEvent.click(sendButton); + fireEvent.click(sendButton); + }); + + await waitFor(() => { + expect(chatSend).toHaveBeenCalledTimes(1); + }); + expect(threadApi.appendMessage).toHaveBeenCalledTimes(1); + expect(chatSend).toHaveBeenCalledWith({ + threadId: thread.id, + message: 'slow backend', + model: 'reasoning-v1', + profileId: 'default', + locale: 'en', + }); + expect(screen.getByRole('button', { name: 'Send message' })).toBeDisabled(); + resolveSend?.(); + }); + + it('releases the pending-send lock when appendMessage rejects with a generic error', async () => { + vi.mocked(threadApi.appendMessage).mockRejectedValueOnce(new Error('disk full')); + const { textarea } = await renderSelectedConversation(); + + await act(async () => { + fireEvent.change(textarea, { target: { value: 'will fail locally' } }); + }); + const sendButton = screen.getByRole('button', { name: 'Send message' }); + await act(async () => { + fireEvent.click(sendButton); + }); + + // chatSend never runs because the local append failed first. + await waitFor(() => { + expect(threadApi.appendMessage).toHaveBeenCalledTimes(1); + }); + expect(chatSend).not.toHaveBeenCalled(); + + // Pending guard released: the user can re-enter text and the send button + // enables again. + await act(async () => { + fireEvent.change(textarea, { target: { value: 'retry' } }); + }); + await waitFor(() => { + expect(screen.getByRole('button', { name: 'Send message' })).not.toBeDisabled(); + }); + }); + + it('releases the pending-send lock when appendMessage hits a stale-thread error', async () => { + vi.mocked(threadApi.appendMessage).mockRejectedValueOnce( + new CoreRpcError('thread missing', 'thread_not_found') + ); + const { textarea } = await renderSelectedConversation(); + + await act(async () => { + fireEvent.change(textarea, { target: { value: 'stale thread send' } }); + }); + const sendButton = screen.getByRole('button', { name: 'Send message' }); + await act(async () => { + fireEvent.click(sendButton); + }); + + await waitFor(() => { + expect(threadApi.appendMessage).toHaveBeenCalledTimes(1); + }); + expect(chatSend).not.toHaveBeenCalled(); + + // Stale-thread branch silently clears the guard; typing must re-enable Send. + await act(async () => { + fireEvent.change(textarea, { target: { value: 'retry' } }); + }); + await waitFor(() => { + expect(screen.getByRole('button', { name: 'Send message' })).not.toBeDisabled(); + }); + }); + + it('clears the pending guard when the 120s silence timer fires', async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + try { + const { textarea } = await renderSelectedConversation(); + + await act(async () => { + fireEvent.change(textarea, { target: { value: 'hang the backend' } }); + }); + const sendButton = screen.getByRole('button', { name: 'Send message' }); + await act(async () => { + fireEvent.click(sendButton); + }); + await waitFor(() => { + expect(chatSend).toHaveBeenCalledTimes(1); + }); + + // Fast-forward past the 120s silence window with no inference signals. + await act(async () => { + await vi.advanceTimersByTimeAsync(120_000); + }); + + // After the safety timeout, typing should re-enable Send — proves the + // pending guard was reset inside the timeout callback. + await act(async () => { + fireEvent.change(textarea, { target: { value: 'retry after timeout' } }); + }); + await waitFor(() => { + expect(screen.getByRole('button', { name: 'Send message' })).not.toBeDisabled(); + }); + } finally { + vi.useRealTimers(); + } + }); + + it('releases the pending-send lock when chatSend rejects', async () => { + vi.mocked(chatSend).mockRejectedValueOnce(new Error('emit failed')); + const { textarea } = await renderSelectedConversation(); + + await act(async () => { + fireEvent.change(textarea, { target: { value: 'doomed send' } }); + }); + await waitFor(() => { + expect(textarea).toHaveValue('doomed send'); + }); + + const sendButton = screen.getByRole('button', { name: 'Send message' }); + await act(async () => { + fireEvent.click(sendButton); + }); + + await waitFor(() => { + expect(chatSend).toHaveBeenCalledTimes(1); + }); + + // After the failed send, typing again should leave the composer enabled so + // the user can retry — proves the pending guard was released. + await act(async () => { + fireEvent.change(textarea, { target: { value: 'retry send' } }); + }); + await waitFor(() => { + expect(screen.getByRole('button', { name: 'Send message' })).not.toBeDisabled(); + }); + }); + it('creates a custom agent profile from the header draft form', async () => { const thread = makeThread({ id: 'profile-thread', title: 'Profile Thread' }); mockGetThreads.mockResolvedValue({ threads: [thread], count: 1 });