From 4983e8e093ed26b670c417ef8a1a5cb68e9d0e52 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 22 Jul 2026 23:07:15 -0700 Subject: [PATCH] fix(mobile): clear native-chat composer optimistically at send time Over relay the send RPC round trip is visible and a lost ack (or a relay/direct cutover) could strand the sent prompt in the composer forever: the unconfirmed-send deadline dropped its tracking entry, so a late transcript echo could never clear the draft. Clear the draft at send time and restore it only on a definite rejection. holdUnconfirmedSend now only manages the delivery-unconfirmed notice; it no longer touches drafts. --- .../use-mobile-native-chat-controller.ts | 11 +- .../use-mobile-native-chat-drafts.test.ts | 116 ++++++++++++++++-- .../session/use-mobile-native-chat-drafts.ts | 39 +++--- 3 files changed, 137 insertions(+), 29 deletions(-) diff --git a/mobile/src/session/use-mobile-native-chat-controller.ts b/mobile/src/session/use-mobile-native-chat-controller.ts index f10f41698f6..6b1ae3f1033 100644 --- a/mobile/src/session/use-mobile-native-chat-controller.ts +++ b/mobile/src/session/use-mobile-native-chat-controller.ts @@ -110,6 +110,8 @@ export function useMobileNativeChatController(args: { setComposerText: setChatComposerText, pending: chatPending, captureSendOrigin, + clearDraftForSend, + restoreRejectedDraft, acceptSend, holdUnconfirmedSend } = useMobileNativeChatDrafts({ @@ -229,6 +231,10 @@ export function useMobileNativeChatController(args: { onSendError('Message not sent (disconnected)') return false } + // Why: empty the composer at send time, not on the ack — over relay the + // round trip is visible, and a lost ack must not strand the sent prompt + // in the box. Only a definite rejection puts the text back. + clearDraftForSend(origin, text) const outcome = await sendMobileNativeChatMessageWithOutcome({ client, terminal: handle, @@ -246,6 +252,7 @@ export function useMobileNativeChatController(args: { return true } if (outcome === 'rejected') { + restoreRejectedDraft(origin, text) onSendError('Message not sent') return false } @@ -256,11 +263,13 @@ export function useMobileNativeChatController(args: { acceptSend, activeHandleRef, captureSendOrigin, + clearDraftForSend, client, deviceTokenRef, holdUnconfirmedSend, nativeChatInputLeaseReady, - onSendError + onSendError, + restoreRejectedDraft ] ) diff --git a/mobile/src/session/use-mobile-native-chat-drafts.test.ts b/mobile/src/session/use-mobile-native-chat-drafts.test.ts index 6ef6f0663a7..3d7042d8dda 100644 --- a/mobile/src/session/use-mobile-native-chat-drafts.test.ts +++ b/mobile/src/session/use-mobile-native-chat-drafts.test.ts @@ -85,6 +85,11 @@ describe('useMobileNativeChatDrafts', () => { act(() => state?.setComposerText('from a')) const originA = state?.captureSendOrigin('from a') expect(originA).not.toBeNull() + act(() => { + if (originA) { + state?.clearDraftForSend(originA, 'from a') + } + }) await switchTo('b') act(() => state?.setComposerText('from b')) @@ -101,6 +106,99 @@ describe('useMobileNativeChatDrafts', () => { expect(state?.pending.map((pending) => pending.text)).toEqual(['from a']) }) + it('clears the composer at send time, before the RPC settles', async () => { + await mount('a') + act(() => state?.setComposerText('ping')) + const origin = state?.captureSendOrigin('ping') + act(() => { + if (origin) { + state?.clearDraftForSend(origin, 'ping') + } + }) + expect(state?.composerText).toBe('') + }) + + it('restores the text on a definite rejection', async () => { + await mount('a') + act(() => state?.setComposerText('ping')) + const origin = state?.captureSendOrigin('ping') + act(() => { + if (origin) { + state?.clearDraftForSend(origin, 'ping') + state?.restoreRejectedDraft(origin, 'ping') + } + }) + expect(state?.composerText).toBe('ping') + }) + + it('does not clobber newer edits when restoring a rejected send', async () => { + await mount('a') + act(() => state?.setComposerText('ping')) + const origin = state?.captureSendOrigin('ping') + act(() => { + if (origin) { + state?.clearDraftForSend(origin, 'ping') + } + }) + act(() => state?.setComposerText('newer edit')) + act(() => { + if (origin) { + state?.restoreRejectedDraft(origin, 'ping') + } + }) + expect(state?.composerText).toBe('newer edit') + }) + + it('restores a rejected send onto its originating tab only', async () => { + await mount('a') + act(() => state?.setComposerText('from a')) + const originA = state?.captureSendOrigin('from a') + act(() => { + if (originA) { + state?.clearDraftForSend(originA, 'from a') + } + }) + + await switchTo('b') + act(() => { + if (originA) { + state?.restoreRejectedDraft(originA, 'from a') + } + }) + expect(state?.composerText).toBe('') + + await switchTo('a') + expect(state?.composerText).toBe('from a') + }) + + it('keeps the composer clear when the echo lands after the unconfirmed deadline', async () => { + vi.useFakeTimers() + try { + await mount('a') + act(() => state?.setComposerText('ping')) + const origin = state?.captureSendOrigin('ping') + act(() => { + if (origin) { + state?.clearDraftForSend(origin, 'ping') + state?.holdUnconfirmedSend(origin, 'ping', vi.fn()) + } + }) + expect(state?.composerText).toBe('') + + // A relay drop can stall the transcript stream past the deadline; the + // delivered prompt must not reappear in the composer when it recovers. + act(() => vi.advanceTimersByTime(25_000)) + await act(async () => + renderer?.update( + createElement(Harness, { tabId: 'a', messages: [userTextMessage('m1', 'ping')] }) + ) + ) + expect(state?.composerText).toBe('') + } finally { + vi.useRealTimers() + } + }) + it('clears one pending per landed message so duplicate sends are not all dropped', async () => { await mount('a') const origin = state?.captureSendOrigin('ping') @@ -159,25 +257,24 @@ describe('useMobileNativeChatDrafts', () => { expect(state?.pending).toEqual([]) }) - it('does not erase newer edits when an older send settles', async () => { + it('does not erase newer edits when an older send clears', async () => { await mount('a') act(() => state?.setComposerText('submitted')) const origin = state?.captureSendOrigin('submitted') act(() => state?.setComposerText('new edit')) act(() => { if (origin) { - state?.acceptSend(origin, 'submitted') + state?.clearDraftForSend(origin, 'submitted') } }) expect(state?.composerText).toBe('new edit') }) - it('clears the draft when an unconfirmed send lands in the transcript', async () => { + it('stays quiet when an unconfirmed send lands in the transcript', async () => { vi.useFakeTimers() try { await mount('a') - act(() => state?.setComposerText('ping')) const origin = state?.captureSendOrigin('ping') const onUnconfirmed = vi.fn() act(() => { @@ -185,14 +282,12 @@ describe('useMobileNativeChatDrafts', () => { state?.holdUnconfirmedSend(origin, 'ping', onUnconfirmed) } }) - expect(state?.composerText).toBe('ping') await act(async () => renderer?.update( createElement(Harness, { tabId: 'a', messages: [userTextMessage('m1', 'ping')] }) ) ) - expect(state?.composerText).toBe('') act(() => vi.advanceTimersByTime(30_000)) expect(onUnconfirmed).not.toHaveBeenCalled() @@ -201,11 +296,10 @@ describe('useMobileNativeChatDrafts', () => { } }) - it('clears immediately when the transcript echo beat the ambiguous RPC rejection', async () => { + it('registers no deadline when the transcript echo beat the ambiguous RPC rejection', async () => { vi.useFakeTimers() try { await mount('a') - act(() => state?.setComposerText('ping')) const origin = state?.captureSendOrigin('ping') const onUnconfirmed = vi.fn() @@ -220,7 +314,6 @@ describe('useMobileNativeChatDrafts', () => { } }) - expect(state?.composerText).toBe('') expect(vi.getTimerCount()).toBe(0) act(() => vi.advanceTimersByTime(30_000)) expect(onUnconfirmed).not.toHaveBeenCalled() @@ -229,11 +322,10 @@ describe('useMobileNativeChatDrafts', () => { } }) - it('surfaces uncertainty and keeps the draft when no echo lands before the deadline', async () => { + it('surfaces uncertainty when no echo lands before the deadline', async () => { vi.useFakeTimers() try { await mount('a') - act(() => state?.setComposerText('ping')) const origin = state?.captureSendOrigin('ping') const onUnconfirmed = vi.fn() act(() => { @@ -246,7 +338,6 @@ describe('useMobileNativeChatDrafts', () => { expect(onUnconfirmed).not.toHaveBeenCalled() act(() => vi.advanceTimersByTime(1)) expect(onUnconfirmed).toHaveBeenCalledTimes(1) - expect(state?.composerText).toBe('ping') } finally { vi.useRealTimers() } @@ -433,6 +524,7 @@ describe('useMobileNativeChatDrafts', () => { expect(origin).toMatchObject({ pendingKey: null }) act(() => { if (origin) { + state?.clearDraftForSend(origin, 'start the session') state?.acceptSend(origin, 'start the session') } }) diff --git a/mobile/src/session/use-mobile-native-chat-drafts.ts b/mobile/src/session/use-mobile-native-chat-drafts.ts index aa954d52699..b0adbf07dd5 100644 --- a/mobile/src/session/use-mobile-native-chat-drafts.ts +++ b/mobile/src/session/use-mobile-native-chat-drafts.ts @@ -99,6 +99,10 @@ export function useMobileNativeChatDrafts(args: { setComposerText: Dispatch> pending: MobileNativeChatPendingMessage[] captureSendOrigin: (text: string) => MobileNativeChatSendOrigin | null + /** Clear the composer at send time, before the RPC settles. */ + clearDraftForSend: (origin: MobileNativeChatSendOrigin, text: string) => void + /** Put the text back after a definite rejection, unless newer edits exist. */ + restoreRejectedDraft: (origin: MobileNativeChatSendOrigin, text: string) => void acceptSend: (origin: MobileNativeChatSendOrigin, text: string) => void holdUnconfirmedSend: ( origin: MobileNativeChatSendOrigin, @@ -154,16 +158,27 @@ export function useMobileNativeChatDrafts(args: { [draftKey, pendingKey] ) - const acceptSend = useCallback((origin: MobileNativeChatSendOrigin, text: string) => { - // Why: an RPC may settle after a tab switch; mutate only the tab that - // originated the send, without erasing edits typed after it began. + // Why: over relay the send RPC can take seconds (or lose only its ack), and a + // composer that waits for settlement to empty reads as "my prompt didn't + // send". Clear at send time; a definite rejection restores the text below. + const clearDraftForSend = useCallback((origin: MobileNativeChatSendOrigin, text: string) => { setDrafts((previous) => (previous[origin.draftKey] ?? '').trim() === text.trim() ? { ...previous, [origin.draftKey]: '' } : previous ) + }, []) + + const restoreRejectedDraft = useCallback((origin: MobileNativeChatSendOrigin, text: string) => { + // Why: never clobber text the user typed while the rejection was in flight. + setDrafts((previous) => + (previous[origin.draftKey] ?? '') === '' ? { ...previous, [origin.draftKey]: text } : previous + ) + }, []) + + const acceptSend = useCallback((origin: MobileNativeChatSendOrigin, text: string) => { // Why: the first prompt can be sent before the provider reports a session - // id; clear its draft, but wait for an id before keying an optimistic echo. + // id; wait for an id before keying an optimistic echo. if (!origin.pendingKey) { return } @@ -187,8 +202,9 @@ export function useMobileNativeChatDrafts(args: { // Why: a relay drop mid-send loses only the ack in the common case — the // desktop already delivered the message. Hold the send instead of claiming - // failure (which baits a duplicate): clear the draft when the transcript echo + // failure (which baits a duplicate): stay quiet when the transcript echo // lands, and surface the uncertainty if the deadline passes without one. + // The composer was already cleared at send time, so this never touches drafts. const unconfirmedRef = useRef([]) const holdUnconfirmedSend = useCallback( (origin: MobileNativeChatSendOrigin, text: string, onUnconfirmed: () => void) => { @@ -211,11 +227,6 @@ export function useMobileNativeChatDrafts(args: { isActiveTranscript && findLandedUnconfirmedSends(messagesRef.current, [entry]).length > 0 ) { - setDrafts((previous) => - (previous[origin.draftKey] ?? '').trim() === text.trim() - ? { ...previous, [origin.draftKey]: '' } - : previous - ) return } entry.deadline = setTimeout(() => { @@ -246,12 +257,6 @@ export function useMobileNativeChatDrafts(args: { if (entry.deadline !== null) { clearTimeout(entry.deadline) } - // Same guard as acceptSend: never erase edits typed after the send began. - setDrafts((previous) => - (previous[entry.draftKey] ?? '').trim() === entry.text.trim() - ? { ...previous, [entry.draftKey]: '' } - : previous - ) } }, [messages, draftKey, pendingKey]) @@ -306,6 +311,8 @@ export function useMobileNativeChatDrafts(args: { setComposerText, pending, captureSendOrigin, + clearDraftForSend, + restoreRejectedDraft, acceptSend, holdUnconfirmedSend }