fix(mobile): keep image attachments scoped to their chat

This commit is contained in:
Brennan Benson
2026-07-22 16:54:31 -07:00
parent 32e258d7fc
commit ac34bdacf8
5 changed files with 234 additions and 17 deletions
@@ -7,6 +7,7 @@ vi.mock('react-native', async () => {
const React = await import('react')
return {
ActivityIndicator: 'ActivityIndicator',
Image: 'Image',
Pressable: 'Pressable',
ScrollView: ({ children, ...props }: { children?: unknown }) =>
React.createElement('ScrollView', props, children),
@@ -24,7 +25,8 @@ vi.mock('lucide-react-native', () => ({
ArrowUp: 'ArrowUp',
ImagePlus: 'ImagePlus',
Mic: 'Mic',
Square: 'Square'
Square: 'Square',
X: 'X'
}))
function suppressRendererWarning(): () => void {
@@ -112,6 +114,54 @@ describe('MobileNativeChatComposer', () => {
expect(onSend).not.toHaveBeenCalled()
})
it('allows an image-only send once its upload is ready', async () => {
const onSend = vi.fn().mockResolvedValue(true)
const restore = suppressRendererWarning()
try {
await act(async () => {
renderer = create(
createElement(MobileNativeChatComposer, {
value: '',
onChangeText: vi.fn(),
onSend,
pendingImages: [{ id: 'image-1', thumbnailUri: 'file:///image.png', status: 'ready' }]
})
)
})
} finally {
restore()
}
expect(sendButton().props).toMatchObject({ disabled: false })
await act(async () => sendButton().props.onPress())
expect(onSend).toHaveBeenCalledWith('')
})
it('blocks send while any pending image is still uploading', async () => {
const onSend = vi.fn().mockResolvedValue(true)
const restore = suppressRendererWarning()
try {
await act(async () => {
renderer = create(
createElement(MobileNativeChatComposer, {
value: 'caption',
onChangeText: vi.fn(),
onSend,
pendingImages: [
{ id: 'image-1', thumbnailUri: 'file:///image.png', status: 'uploading' }
]
})
)
})
} finally {
restore()
}
expect(sendButton().props).toMatchObject({ disabled: true })
await act(async () => sendButton().props.onPress())
expect(onSend).not.toHaveBeenCalled()
})
it('moves the caret to the insert point after an autocomplete pick, then releases control', async () => {
const restore = suppressRendererWarning()
try {
@@ -13,6 +13,7 @@ const onSendError = vi.fn()
let readyImages: readonly MobileNativeChatReadyImage[] = []
let leaseReady = true
let activeHandle = 'term-1'
describe('useMobileNativeChatImageSend', () => {
let renderer: ReactTestRenderer | null = null
@@ -23,6 +24,7 @@ describe('useMobileNativeChatImageSend', () => {
vi.clearAllMocks()
readyImages = []
leaseReady = true
activeHandle = 'term-1'
sendRequest.mockResolvedValue({ ok: true, result: { send: { accepted: true } } })
sendText.mockResolvedValue(true)
})
@@ -36,7 +38,11 @@ describe('useMobileNativeChatImageSend', () => {
function Harness(): null {
send = useMobileNativeChatImageSend({
clientRef: { current: client },
activeHandleRef: { current: 'term-1' },
activeHandleRef: {
get current() {
return activeHandle
}
},
deviceTokenRef: { current: 'device-9' },
inputLeaseReadyRef: {
get current() {
@@ -130,4 +136,36 @@ describe('useMobileNativeChatImageSend', () => {
expect(consumeImages).not.toHaveBeenCalled()
expect(onSendError).toHaveBeenCalledWith('Message not sent (disconnected)')
})
it('does not send the caption to a terminal selected while image paths were being pasted', async () => {
readyImages = [{ id: 'a', hostPath: '/tmp/a.png' }]
sendRequest.mockImplementation(async () => {
activeHandle = 'term-2'
return { ok: true, result: { send: { accepted: true } } }
})
render()
await expect(send!('caption')).resolves.toBe(false)
expect(sendRequest).toHaveBeenCalledTimes(1)
expect(consumeImages).toHaveBeenCalledWith(['a'])
expect(sendText).not.toHaveBeenCalled()
expect(onSendError).toHaveBeenCalledWith('Message not sent (disconnected)')
})
it('does not continue pasting images after the target terminal changes', async () => {
readyImages = [
{ id: 'a', hostPath: '/tmp/a.png' },
{ id: 'b', hostPath: '/tmp/b.png' }
]
sendRequest.mockImplementation(async () => {
activeHandle = 'term-2'
return { ok: true, result: { send: { accepted: true } } }
})
render()
await expect(send!('caption')).resolves.toBe(false)
expect(sendRequest).toHaveBeenCalledTimes(1)
expect(consumeImages.mock.calls).toEqual([[['a']]])
expect(sendText).not.toHaveBeenCalled()
})
})
@@ -46,10 +46,18 @@ export function useMobileNativeChatImageSend({
onSendError('Message not sent (disconnected)')
return false
}
const targetIsCurrent = (): boolean =>
clientRef.current === client &&
activeHandleRef.current === terminal &&
inputLeaseReadyRef.current
const mobileClient = deviceTokenRef.current
? { id: deviceTokenRef.current, type: 'mobile' as const }
: undefined
for (const image of images) {
if (!targetIsCurrent()) {
onSendError('Message not sent (disconnected)')
return false
}
const accepted = await sendMobileNativeChatMessage({
client,
terminal,
@@ -66,9 +74,19 @@ export function useMobileNativeChatImageSend({
consumeImages([image.id])
}
if (text.trim().length > 0) {
// 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
}
return sendText(text)
}
// Image-only send: the paths are in the TUI input; submit with Enter.
if (!targetIsCurrent()) {
onSendError('Message not sent (disconnected)')
return false
}
const submitted = await sendMobileNativeChatMessage({
client,
terminal,
@@ -168,4 +168,81 @@ describe('useMobileNativeChatPendingImages', () => {
expect(state!.pendingChatImages).toEqual([])
expect(state!.getReadyChatImages()).toEqual([])
})
it('does not attach a picker result to a terminal selected while the picker was open', async () => {
let resolvePick: (image: { base64: string; uri: string }) => void = () => {}
pickMobileImage.mockReturnValue(
new Promise((resolve) => {
resolvePick = resolve
})
)
render('t1')
let attachDone: Promise<void> = Promise.resolve()
act(() => {
attachDone = state!.attachPendingChatImage('library')
renderer!.update(createElement(Harness, { activeHandle: 't2' }))
})
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 = () => {}
saveMobileClipboardImageAsTempFile.mockReturnValue(
new Promise<string>((resolve) => {
resolveUpload = resolve
})
)
render()
let attachDone: Promise<void> = Promise.resolve()
await act(async () => {
attachDone = state!.attachPendingChatImage('library')
await Promise.resolve()
await Promise.resolve()
})
act(() => state!.removePendingChatImage('chat-image-1'))
await act(async () => {
resolveUpload('/tmp/a.png')
await attachDone
})
expect(state!.pendingChatImages).toEqual([])
expect(onSuccess).not.toHaveBeenCalled()
})
it('releases pending state and ignores upload completion after unmount', async () => {
pickMobileImage.mockResolvedValue({ base64: 'AAAA', uri: 'file:///a.jpg' })
let resolveUpload: (path: string) => void = () => {}
saveMobileClipboardImageAsTempFile.mockReturnValue(
new Promise<string>((resolve) => {
resolveUpload = resolve
})
)
render()
let attachDone: Promise<void> = Promise.resolve()
await act(async () => {
attachDone = state!.attachPendingChatImage('library')
await Promise.resolve()
await Promise.resolve()
})
act(() => renderer!.unmount())
renderer = null
await act(async () => {
resolveUpload('/tmp/a.png')
await attachDone
})
expect(onSuccess).not.toHaveBeenCalled()
expect(onError).not.toHaveBeenCalled()
})
})
@@ -1,4 +1,4 @@
import { useCallback, useRef, useState } from 'react'
import { useCallback, useEffect, useRef, useState } from 'react'
import type { RpcClient } from '../transport/rpc-client'
import type { ConnectionState } from '../transport/types'
import { saveMobileClipboardImageAsTempFile } from './mobile-clipboard-image'
@@ -58,14 +58,26 @@ export function useMobileNativeChatPendingImages({
const [entries, setEntries] = useState<readonly PendingImageEntry[]>([])
const entriesRef = useRef(entries)
const idCounter = useRef(0)
const mountedRef = useRef(false)
useEffect(() => {
mountedRef.current = true
return () => {
mountedRef.current = false
// Why: release thumbnail data captured by in-flight uploads instead of retaining it until RPC timeout.
entriesRef.current = []
}
}, [])
const updateEntries = useCallback(
(updater: (previous: readonly PendingImageEntry[]) => readonly PendingImageEntry[]) => {
setEntries((previous) => {
const next = updater(previous)
entriesRef.current = next
return next
})
const previous = entriesRef.current
const next = updater(previous)
if (next === previous) {
return
}
entriesRef.current = next
setEntries(next)
},
[]
)
@@ -87,11 +99,12 @@ export function useMobileNativeChatPendingImages({
if (!client || !activeHandle || !canAttach) {
return
}
const targetHandle = activeHandle
let entryId: string | null = null
try {
const picked = await pickMobileImage(source)
// Cancelled picker: no entry, no toast.
if (!picked) {
if (!picked || !mountedRef.current || lastHandle.current !== targetHandle) {
return
}
idCounter.current += 1
@@ -109,18 +122,39 @@ export function useMobileNativeChatPendingImages({
}
])
const connectionId = await getConnectionId()
if (!mountedRef.current || lastHandle.current !== targetHandle) {
return
}
const hostPath = await saveMobileClipboardImageAsTempFile(client, picked.base64, {
connectionId
})
// A handle switch mid-upload cleared the entry; the orphaned host temp
// file is covered by the upload TTL.
updateEntries((previous) =>
previous.map((entry) =>
entry.id === id ? { ...entry, status: 'ready' as const, hostPath } : entry
)
)
onSuccess()
if (!mountedRef.current || lastHandle.current !== targetHandle) {
return
}
let completed = false
updateEntries((previous) => {
const index = previous.findIndex((entry) => entry.id === id)
if (index < 0) {
return previous
}
completed = true
const next = previous.slice()
next[index] = { ...previous[index], status: 'ready', hostPath }
return next
})
if (completed) {
onSuccess()
}
} catch (error) {
// Why: switching targets or removing the tile cancels this attachment;
// late failures must not surface against the current composer.
if (
!mountedRef.current ||
lastHandle.current !== targetHandle ||
(entryId !== null && !entriesRef.current.some((entry) => entry.id === entryId))
) {
return
}
if (entryId !== null) {
const failedId = entryId
updateEntries((previous) => previous.filter((entry) => entry.id !== failedId))