fix(mobile): cancel stale image attachment work

This commit is contained in:
Brennan Benson
2026-07-22 21:10:35 -07:00
parent 7376a77000
commit 4ad54c0413
6 changed files with 153 additions and 26 deletions
@@ -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<string | null> => {
// 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
@@ -70,6 +70,7 @@ export function useMobileNativeChatImageAttachments({
activeHandleRef,
deviceTokenRef,
inputLeaseReadyRef,
attachmentScopeKey,
sendText,
getSendableImages: getSendableChatImages,
markImagesPasted: markPendingChatImagesPasted,
@@ -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<boolean> {
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()
})
})
@@ -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<string | null>
readonly deviceTokenRef: CurrentRef<string | null>
readonly inputLeaseReadyRef: CurrentRef<boolean>
readonly attachmentScopeKey: string
/** The plain text send (controller-owned: optimistic bubble + draft clear). */
readonly sendText: (text: string) => Promise<boolean>
readonly getSendableImages: () => readonly MobileNativeChatSendImage[]
@@ -38,12 +39,36 @@ export function useMobileNativeChatImageSend({
activeHandleRef,
deviceTokenRef,
inputLeaseReadyRef,
attachmentScopeKey,
sendText,
getSendableImages,
markImagesPasted,
consumeImages,
onSendError
}: UseMobileNativeChatImageSendArgs): (text: string) => Promise<boolean> {
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<boolean> => {
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,
@@ -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<void> = 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 = () => {}
@@ -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 {