diff --git a/mobile/app/h/[hostId]/session/[worktreeId].tsx b/mobile/app/h/[hostId]/session/[worktreeId].tsx index a947db0ec93..2d07bdf3f8b 100644 --- a/mobile/app/h/[hostId]/session/[worktreeId].tsx +++ b/mobile/app/h/[hostId]/session/[worktreeId].tsx @@ -1126,10 +1126,7 @@ export default function SessionScreen() { }, [clearToastHideTimer] ) - const showNativeChatSendError = useCallback( - (message: string) => showToast(message, 1600), - [showToast] - ) + const showChatSendError = useCallback((message: string) => showToast(message, 1600), [showToast]) const getActiveWorktreeConnectionId = useCallback(async (): Promise => { // Why: the floating workspace always runs on the paired host itself, never an SSH repo target. if (!client || isFloatingWorkspaceRoute) { @@ -1174,7 +1171,7 @@ export default function SessionScreen() { }, nativeChatTranscriptIsLocalReadable, nativeChatInputLeaseReady, - onSendError: showNativeChatSendError + onSendError: showChatSendError }) const { toggleTabChatView, showNativeChat, showNativeChatRef } = nativeChatController diff --git a/mobile/src/session/use-mobile-native-chat-image-attachments.ts b/mobile/src/session/use-mobile-native-chat-image-attachments.ts index 55014dc297d..9c1826bc0b9 100644 --- a/mobile/src/session/use-mobile-native-chat-image-attachments.ts +++ b/mobile/src/session/use-mobile-native-chat-image-attachments.ts @@ -70,6 +70,7 @@ export function useMobileNativeChatImageAttachments({ activeHandleRef, deviceTokenRef, inputLeaseReadyRef, + attachmentScopeKey, sendText, getSendableImages: getSendableChatImages, markImagesPasted: markPendingChatImagesPasted, diff --git a/mobile/src/session/use-mobile-native-chat-image-send.test.ts b/mobile/src/session/use-mobile-native-chat-image-send.test.ts index 41ce22dbed7..5fca522b724 100644 --- a/mobile/src/session/use-mobile-native-chat-image-send.test.ts +++ b/mobile/src/session/use-mobile-native-chat-image-send.test.ts @@ -20,6 +20,7 @@ const onSendError = vi.fn() let sendableImages: MobileNativeChatSendImage[] = [] let leaseReady = true let activeHandle = 'term-1' +let attachmentScopeKey = 'scope-1' describe('useMobileNativeChatImageSend', () => { let renderer: ReactTestRenderer | null = null @@ -32,6 +33,7 @@ describe('useMobileNativeChatImageSend', () => { sendableImages = [] leaseReady = true activeHandle = 'term-1' + attachmentScopeKey = 'scope-1' sendRequest.mockResolvedValue({ ok: true, result: { send: { accepted: true } } }) sendText.mockResolvedValue(true) markImagesPasted.mockImplementation((ids: readonly string[]) => { @@ -67,6 +69,7 @@ describe('useMobileNativeChatImageSend', () => { return leaseReady } }, + attachmentScopeKey, sendText, getSendableImages: () => sendableImages, markImagesPasted, @@ -82,6 +85,13 @@ describe('useMobileNativeChatImageSend', () => { }) } + function switchScope(scopeKey: string): void { + attachmentScopeKey = scopeKey + act(() => { + renderer!.update(createElement(Harness)) + }) + } + async function sendAndRunTimers(text: string): Promise { const result = send!(text) await vi.runAllTimersAsync() @@ -198,6 +208,28 @@ describe('useMobileNativeChatImageSend', () => { expect(sendText).not.toHaveBeenCalled() }) + it('does not resume an in-flight send after switching away and back to the same target', async () => { + sendableImages = [{ id: 'a', status: 'ready', hostPath: '/tmp/a.png' }] + let resolvePaste: (result: unknown) => void = () => {} + sendRequest.mockReturnValue( + new Promise((resolve) => { + resolvePaste = resolve + }) + ) + render() + + const sendResult = send!('caption') + await vi.advanceTimersByTimeAsync(0) + switchScope('scope-2') + switchScope('scope-1') + resolvePaste({ ok: true, result: { send: { accepted: true } } }) + + await expect(sendResult).resolves.toBe(false) + expect(markImagesPasted).toHaveBeenCalledWith(['a']) + expect(sendText).not.toHaveBeenCalled() + expect(onSendError).toHaveBeenCalledWith('Message not sent (disconnected)') + }) + it('preserves ambiguous image state without reporting definite failure or pasting twice', async () => { sendableImages = [{ id: 'a', status: 'ready', hostPath: '/tmp/a.png' }] sendRequest.mockRejectedValueOnce(markRpcDeliveryUnknown(new Error('ack lost'))) @@ -271,4 +303,20 @@ describe('useMobileNativeChatImageSend', () => { await expect(sendResult).resolves.toBe(true) expect(sendRequest).toHaveBeenCalledTimes(2) }) + + it('does not continue a delayed send or report against UI after unmount', async () => { + sendableImages = [{ id: 'a', status: 'ready', hostPath: '/tmp/a.png' }] + render() + + const sendResult = send!('caption') + await vi.advanceTimersByTimeAsync(0) + expect(sendRequest).toHaveBeenCalledTimes(1) + act(() => renderer!.unmount()) + renderer = null + await vi.runAllTimersAsync() + + await expect(sendResult).resolves.toBe(false) + expect(sendText).not.toHaveBeenCalled() + expect(onSendError).not.toHaveBeenCalled() + }) }) diff --git a/mobile/src/session/use-mobile-native-chat-image-send.ts b/mobile/src/session/use-mobile-native-chat-image-send.ts index 80ba93d7bae..35a2de68f8a 100644 --- a/mobile/src/session/use-mobile-native-chat-image-send.ts +++ b/mobile/src/session/use-mobile-native-chat-image-send.ts @@ -1,4 +1,4 @@ -import { useCallback } from 'react' +import { useCallback, useEffect, useRef } from 'react' import { NATIVE_CHAT_IMAGE_ATTACHMENT_SETTLE_MS, NATIVE_CHAT_SUBMIT_DELAY_MS @@ -17,6 +17,7 @@ type UseMobileNativeChatImageSendArgs = { readonly activeHandleRef: CurrentRef readonly deviceTokenRef: CurrentRef readonly inputLeaseReadyRef: CurrentRef + readonly attachmentScopeKey: string /** The plain text send (controller-owned: optimistic bubble + draft clear). */ readonly sendText: (text: string) => Promise readonly getSendableImages: () => readonly MobileNativeChatSendImage[] @@ -38,12 +39,36 @@ export function useMobileNativeChatImageSend({ activeHandleRef, deviceTokenRef, inputLeaseReadyRef, + attachmentScopeKey, sendText, getSendableImages, markImagesPasted, consumeImages, onSendError }: UseMobileNativeChatImageSendArgs): (text: string) => Promise { + const mountedRef = useRef(false) + const targetStateRef = useRef({ + client: clientRef.current, + attachmentScopeKey, + generation: 0 + }) + if ( + targetStateRef.current.client !== clientRef.current || + targetStateRef.current.attachmentScopeKey !== attachmentScopeKey + ) { + targetStateRef.current = { + client: clientRef.current, + attachmentScopeKey, + generation: targetStateRef.current.generation + 1 + } + } + useEffect(() => { + mountedRef.current = true + return () => { + mountedRef.current = false + } + }, []) + return useCallback( async (text: string): Promise => { const images = getSendableImages() @@ -53,14 +78,26 @@ export function useMobileNativeChatImageSend({ const hasText = text.trim().length > 0 const client = clientRef.current const terminal = activeHandleRef.current + const targetGeneration = targetStateRef.current.generation + if (!mountedRef.current) { + return false + } if (!client || !terminal || !inputLeaseReadyRef.current) { onSendError('Message not sent (disconnected)') return false } - const targetIsCurrent = (): boolean => + const targetIdentityIsCurrent = (): boolean => + mountedRef.current && + targetStateRef.current.generation === targetGeneration && clientRef.current === client && - activeHandleRef.current === terminal && - inputLeaseReadyRef.current + activeHandleRef.current === terminal + const targetIsReady = (): boolean => targetIdentityIsCurrent() && inputLeaseReadyRef.current + const reportTargetChanged = (): false => { + if (mountedRef.current) { + onSendError('Message not sent (disconnected)') + } + return false + } const mobileClient = deviceTokenRef.current ? { id: deviceTokenRef.current, type: 'mobile' as const } : undefined @@ -72,9 +109,8 @@ export function useMobileNativeChatImageSend({ if (image.status === 'pasted') { continue } - if (!targetIsCurrent()) { - onSendError('Message not sent (disconnected)') - return false + if (!targetIsReady()) { + return reportTargetChanged() } const outcome = await sendMobileNativeChatMessageWithOutcome({ client, @@ -84,7 +120,9 @@ export function useMobileNativeChatImageSend({ ...(mobileClient ? { mobileClient } : {}) }) if (outcome === 'rejected') { - onSendError('Message not sent') + if (targetIdentityIsCurrent()) { + onSendError('Message not sent') + } return false } // Why: keep paths already written to the hidden TUI distinct from ready @@ -92,10 +130,16 @@ export function useMobileNativeChatImageSend({ markImagesPasted([image.id]) pastedImageIds.push(image.id) pastedImageThisSend = true + if (!targetIdentityIsCurrent()) { + return reportTargetChanged() + } if (outcome === 'unknown') { await waitForNativeChatInputSettle( hasText ? NATIVE_CHAT_IMAGE_ATTACHMENT_SETTLE_MS : NATIVE_CHAT_SUBMIT_DELAY_MS ) + if (!targetIdentityIsCurrent()) { + return false + } onSendError('Image delivery unconfirmed — check terminal before retrying') // Match text-send ambiguity: preserve retry state without also showing // the contradictory inline "Message not sent" failure. @@ -108,12 +152,11 @@ export function useMobileNativeChatImageSend({ } // Why: image RPCs can outlive a tab switch; never send the caption to a // different terminal than the one that received the image paths. - if (!targetIsCurrent()) { - onSendError('Message not sent (disconnected)') - return false + if (!targetIsReady()) { + return reportTargetChanged() } const accepted = await sendText(text) - if (accepted) { + if (accepted && targetIdentityIsCurrent()) { consumeImages(pastedImageIds) } return accepted @@ -122,9 +165,8 @@ export function useMobileNativeChatImageSend({ if (pastedImageThisSend) { await waitForNativeChatInputSettle(NATIVE_CHAT_SUBMIT_DELAY_MS) } - if (!targetIsCurrent()) { - onSendError('Message not sent (disconnected)') - return false + if (!targetIsReady()) { + return reportTargetChanged() } const outcome = await sendMobileNativeChatMessageWithOutcome({ client, @@ -133,6 +175,9 @@ export function useMobileNativeChatImageSend({ enter: true, ...(mobileClient ? { mobileClient } : {}) }) + if (!targetIdentityIsCurrent()) { + return false + } if (outcome === 'rejected') { onSendError('Message not sent') return false @@ -145,6 +190,7 @@ export function useMobileNativeChatImageSend({ }, [ activeHandleRef, + attachmentScopeKey, clientRef, consumeImages, deviceTokenRef, diff --git a/mobile/src/session/use-mobile-native-chat-pending-images.test.ts b/mobile/src/session/use-mobile-native-chat-pending-images.test.ts index 0d85abb485c..9431dae6a51 100644 --- a/mobile/src/session/use-mobile-native-chat-pending-images.test.ts +++ b/mobile/src/session/use-mobile-native-chat-pending-images.test.ts @@ -243,6 +243,35 @@ describe('useMobileNativeChatPendingImages', () => { expect(onSuccess).not.toHaveBeenCalled() }) + it('does not attach a picker result after switching away and back to the same target', async () => { + let resolvePick: (image: { base64: string; uri: string }) => void = () => {} + pickMobileImage.mockReturnValue( + new Promise((resolve) => { + resolvePick = resolve + }) + ) + render('t1') + + let attachDone: Promise = Promise.resolve() + act(() => { + attachDone = state!.attachPendingChatImage('library') + }) + act(() => { + renderer!.update(createElement(Harness, { activeHandle: 't2' })) + }) + act(() => { + renderer!.update(createElement(Harness, { activeHandle: 't1' })) + }) + await act(async () => { + resolvePick({ base64: 'AAAA', uri: 'file:///a.jpg' }) + await attachDone + }) + + expect(state!.pendingChatImages).toEqual([]) + expect(saveMobileClipboardImageAsTempFile).not.toHaveBeenCalled() + expect(onSuccess).not.toHaveBeenCalled() + }) + it('does not report upload success after the pending image was removed', async () => { pickMobileImage.mockResolvedValue({ base64: 'AAAA', uri: 'file:///a.jpg' }) let resolveUpload: (path: string) => void = () => {} diff --git a/mobile/src/session/use-mobile-native-chat-pending-images.ts b/mobile/src/session/use-mobile-native-chat-pending-images.ts index 64308332364..e47f12f0e70 100644 --- a/mobile/src/session/use-mobile-native-chat-pending-images.ts +++ b/mobile/src/session/use-mobile-native-chat-pending-images.ts @@ -105,12 +105,16 @@ export function useMobileNativeChatPendingImages({ // Why: pending images target the active terminal's agent; a tab/worktree // switch would silently re-aim them, so drop instead (adjust during render, // not in an effect, so a send in the same frame can't read stale entries). - const lastTarget = useRef({ client, attachmentScopeKey }) + const targetState = useRef({ client, attachmentScopeKey, generation: 0 }) if ( - lastTarget.current.client !== client || - lastTarget.current.attachmentScopeKey !== attachmentScopeKey + targetState.current.client !== client || + targetState.current.attachmentScopeKey !== attachmentScopeKey ) { - lastTarget.current = { client, attachmentScopeKey } + targetState.current = { + client, + attachmentScopeKey, + generation: targetState.current.generation + 1 + } if (entriesRef.current.length > 0) { entriesRef.current = [] setEntries([]) @@ -124,10 +128,12 @@ export function useMobileNativeChatPendingImages({ } const targetClient = client const targetScopeKey = attachmentScopeKey + const targetGeneration = targetState.current.generation const targetIsCurrent = (): boolean => mountedRef.current && - lastTarget.current.client === targetClient && - lastTarget.current.attachmentScopeKey === targetScopeKey + targetState.current.client === targetClient && + targetState.current.attachmentScopeKey === targetScopeKey && + targetState.current.generation === targetGeneration let entryId: string | null = null let uploadController: AbortController | null = null try {