fix(mobile): heal terminal input after ambiguous image-send delivery (#10325)
An image send whose text+Enter RPC ended 'unknown' (ack loss / path cutover) collapsed to accepted=true, so the terminal was never marked stale. When the Enter truly never landed, the already-pasted image path sat on the input line and glued onto the next plain-text message. Propagate the send outcome through handleNativeChatSendWithOutcome and mark the terminal input stale on any non-accepted outcome; the next send heals with Ctrl+U (a no-op when the message did land). Chips still clear on 'unknown' to avoid a double-send on retry.
This commit is contained in:
parent
d50ea090cf
commit
e651fe91c6
|
|
@ -3631,7 +3631,7 @@ export default function SessionScreen() {
|
|||
nativeChatInputLeaseReady,
|
||||
getActiveWorktreeConnectionId,
|
||||
beforeTerminalSend: flushPendingLiveInputBeforeAttachmentSend,
|
||||
nativeChatBaseSend: nativeChatController.handleNativeChatSend,
|
||||
nativeChatBaseSend: nativeChatController.handleNativeChatSendWithOutcome,
|
||||
showToast,
|
||||
onSuccess: triggerSelection,
|
||||
onError: triggerError
|
||||
|
|
|
|||
|
|
@ -127,6 +127,18 @@ describe('useMobileNativeChatController handleNativeChatSend', () => {
|
|||
expect(holdUnconfirmedSend).toHaveBeenCalledWith(ORIGIN, 'look', expect.any(Function))
|
||||
})
|
||||
|
||||
it('preserves the unknown outcome on the WithOutcome surface for paste-first callers', async () => {
|
||||
sendWithOutcome.mockResolvedValue('unknown')
|
||||
let outcome = 'accepted'
|
||||
await act(async () => {
|
||||
outcome = await controller!.handleNativeChatSendWithOutcome('look', ['file:///a.jpg'])
|
||||
})
|
||||
// Image sends heal a possibly-orphaned paste off this — 'unknown' must not
|
||||
// collapse into the boolean 'sent' shape (#10228).
|
||||
expect(outcome).toBe('unknown')
|
||||
expect(holdUnconfirmedSend).toHaveBeenCalledWith(ORIGIN, 'look', expect.any(Function))
|
||||
})
|
||||
|
||||
it('reports a rejected send and posts no echo', async () => {
|
||||
sendWithOutcome.mockResolvedValue('rejected')
|
||||
let accepted = true
|
||||
|
|
|
|||
|
|
@ -17,7 +17,10 @@ import { detectAgentPermission } from './mobile-native-chat-permission'
|
|||
import { parseAgentQuestion } from './mobile-native-chat-question'
|
||||
import { openMobileNativeChatFile } from './mobile-native-chat-open-file'
|
||||
import { useMobileNativeChatPermissionSend } from './mobile-native-chat-permission-send'
|
||||
import { sendMobileNativeChatMessageWithOutcome } from './mobile-native-chat-send'
|
||||
import {
|
||||
sendMobileNativeChatMessageWithOutcome,
|
||||
type MobileNativeChatSendOutcome
|
||||
} from './mobile-native-chat-send'
|
||||
import { useMobileNativeChatAnswerSend } from './use-mobile-native-chat-answer-send'
|
||||
import {
|
||||
useMobileNativeChatDrafts,
|
||||
|
|
@ -59,6 +62,12 @@ export type MobileNativeChatController = {
|
|||
nativeChatFilePaths: string[]
|
||||
loadNativeChatFiles: (query: string) => void
|
||||
handleNativeChatSend: (text: string, images?: string[]) => Promise<boolean>
|
||||
/** Outcome-preserving send: callers that pasted terminal input beforehand
|
||||
* (image sends) must see 'unknown' to heal a possibly-orphaned paste. */
|
||||
handleNativeChatSendWithOutcome: (
|
||||
text: string,
|
||||
images?: string[]
|
||||
) => Promise<MobileNativeChatSendOutcome>
|
||||
}
|
||||
|
||||
/** Owns mobile native-chat state and teardown outside the already dense session
|
||||
|
|
@ -224,13 +233,13 @@ export function useMobileNativeChatController(args: {
|
|||
worktreeId
|
||||
})
|
||||
|
||||
const handleNativeChatSend = useCallback(
|
||||
async (text: string, images?: string[]): Promise<boolean> => {
|
||||
const handleNativeChatSendWithOutcome = useCallback(
|
||||
async (text: string, images?: string[]): Promise<MobileNativeChatSendOutcome> => {
|
||||
const handle = activeHandleRef.current
|
||||
const origin = captureSendOrigin(text)
|
||||
if (!client || !handle || !origin || !nativeChatInputLeaseReady) {
|
||||
onSendError('Message not sent (disconnected)')
|
||||
return false
|
||||
return 'rejected'
|
||||
}
|
||||
const outcome = await sendMobileNativeChatMessageWithOutcome({
|
||||
client,
|
||||
|
|
@ -246,16 +255,16 @@ export function useMobileNativeChatController(args: {
|
|||
holdUnconfirmedSend(origin, text, () =>
|
||||
onSendError('Delivery unconfirmed — check chat before retrying')
|
||||
)
|
||||
return true
|
||||
return 'unknown'
|
||||
}
|
||||
if (outcome === 'rejected') {
|
||||
onSendError('Message not sent')
|
||||
return false
|
||||
return 'rejected'
|
||||
}
|
||||
// `images` are local preview URIs for the optimistic echo only — the actual
|
||||
// image bytes already rode along as a bracketed paste before this text send.
|
||||
acceptSend(origin, text, images)
|
||||
return true
|
||||
return 'accepted'
|
||||
},
|
||||
[
|
||||
acceptSend,
|
||||
|
|
@ -269,6 +278,14 @@ export function useMobileNativeChatController(args: {
|
|||
]
|
||||
)
|
||||
|
||||
// Boolean surface for callers with no pre-pasted input: 'unknown' stays true
|
||||
// (the send usually landed; the optimistic echo is already held unconfirmed).
|
||||
const handleNativeChatSend = useCallback(
|
||||
async (text: string, images?: string[]): Promise<boolean> =>
|
||||
(await handleNativeChatSendWithOutcome(text, images)) !== 'rejected',
|
||||
[handleNativeChatSendWithOutcome]
|
||||
)
|
||||
|
||||
return {
|
||||
isTabChatView,
|
||||
toggleTabChatView,
|
||||
|
|
@ -291,6 +308,7 @@ export function useMobileNativeChatController(args: {
|
|||
handleNativeChatStop,
|
||||
nativeChatFilePaths,
|
||||
loadNativeChatFiles,
|
||||
handleNativeChatSend
|
||||
handleNativeChatSend,
|
||||
handleNativeChatSendWithOutcome
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -66,7 +66,7 @@ function baseArgs(overrides: Partial<HookArgs> & Pick<HookArgs, 'client'>): Hook
|
|||
scopeKey: SCOPE_A,
|
||||
enabled: true,
|
||||
showToast: vi.fn(),
|
||||
baseSend: vi.fn().mockResolvedValue(true),
|
||||
baseSend: vi.fn().mockResolvedValue('accepted'),
|
||||
sleep: async () => {},
|
||||
...overrides
|
||||
}
|
||||
|
|
@ -149,7 +149,7 @@ describe('useMobileNativeChatImageAttachments', () => {
|
|||
})
|
||||
const baseSend = vi.fn(async (t: string) => {
|
||||
order.push(`text:${t}`)
|
||||
return true
|
||||
return 'accepted' as const
|
||||
})
|
||||
// Record each terminal write so the paste-before-settle order is asserted,
|
||||
// not just implied by the call counts.
|
||||
|
|
@ -204,7 +204,7 @@ describe('useMobileNativeChatImageAttachments', () => {
|
|||
sendResult(true), // Ctrl+U clear
|
||||
sendResult(true) // image paste
|
||||
])
|
||||
const baseSend = vi.fn().mockResolvedValue(true)
|
||||
const baseSend = vi.fn().mockResolvedValue('accepted')
|
||||
mount(baseArgs({ client: client as unknown as RpcClient, baseSend }))
|
||||
|
||||
await act(async () => {
|
||||
|
|
@ -227,7 +227,7 @@ describe('useMobileNativeChatImageAttachments', () => {
|
|||
|
||||
it('delegates straight to baseSend when there are no attachments', async () => {
|
||||
const client = makeClient([])
|
||||
const baseSend = vi.fn().mockResolvedValue(true)
|
||||
const baseSend = vi.fn().mockResolvedValue('accepted')
|
||||
mount(baseArgs({ client: client as unknown as RpcClient, baseSend }))
|
||||
|
||||
await act(async () => {
|
||||
|
|
@ -245,7 +245,7 @@ describe('useMobileNativeChatImageAttachments', () => {
|
|||
sendResult(true), // Ctrl+U clear
|
||||
sendResult(false) // image paste rejected
|
||||
])
|
||||
const baseSend = vi.fn().mockResolvedValue(true)
|
||||
const baseSend = vi.fn().mockResolvedValue('accepted')
|
||||
const showToast = vi.fn()
|
||||
mount(baseArgs({ client: client as unknown as RpcClient, baseSend, showToast }))
|
||||
await act(async () => {
|
||||
|
|
@ -265,7 +265,7 @@ describe('useMobileNativeChatImageAttachments', () => {
|
|||
pick.mockResolvedValue({ base64: 'AAAA', uri: 'file:///a.jpg' })
|
||||
// No terminal.send responses queued: the clear write throws (dropped transport).
|
||||
const client = makeClient([methodNotFound('start'), ok('save', '/tmp/a.png')])
|
||||
const baseSend = vi.fn().mockResolvedValue(true)
|
||||
const baseSend = vi.fn().mockResolvedValue('accepted')
|
||||
const showToast = vi.fn()
|
||||
mount(baseArgs({ client: client as unknown as RpcClient, baseSend, showToast }))
|
||||
await act(async () => {
|
||||
|
|
@ -284,7 +284,7 @@ describe('useMobileNativeChatImageAttachments', () => {
|
|||
it('surfaces a toast instead of a silent no-op when the input lease gate is closed', async () => {
|
||||
pick.mockResolvedValue({ base64: 'AAAA', uri: 'file:///a.jpg' })
|
||||
const client = makeClient([methodNotFound('start'), ok('save', '/tmp/a.png')])
|
||||
const baseSend = vi.fn().mockResolvedValue(true)
|
||||
const baseSend = vi.fn().mockResolvedValue('accepted')
|
||||
const showToast = vi.fn()
|
||||
// Attaching is allowed without the lease; only the send is gated on it.
|
||||
mount(baseArgs({ client: client as unknown as RpcClient, enabled: false, baseSend, showToast }))
|
||||
|
|
@ -304,7 +304,7 @@ describe('useMobileNativeChatImageAttachments', () => {
|
|||
it('scopes chips to the tab that attached them', async () => {
|
||||
pick.mockResolvedValue({ base64: 'AAAA', uri: 'file:///a.jpg' })
|
||||
const client = makeClient([methodNotFound('start'), ok('save', '/tmp/a.png')])
|
||||
const baseSend = vi.fn().mockResolvedValue(true)
|
||||
const baseSend = vi.fn().mockResolvedValue('accepted')
|
||||
const args = baseArgs({ client: client as unknown as RpcClient, baseSend })
|
||||
mount(args)
|
||||
await act(async () => {
|
||||
|
|
@ -378,7 +378,7 @@ describe('useMobileNativeChatImageAttachments', () => {
|
|||
methodNotFound('start'),
|
||||
ok('save', '/tmp/b.png') // second attach, while the send is parked on settle
|
||||
])
|
||||
const baseSend = vi.fn().mockResolvedValue(true)
|
||||
const baseSend = vi.fn().mockResolvedValue('accepted')
|
||||
let releaseSettle: (() => void) | null = null
|
||||
const args = baseArgs({
|
||||
client: client as unknown as RpcClient,
|
||||
|
|
@ -433,7 +433,7 @@ describe('useMobileNativeChatImageAttachments', () => {
|
|||
sendResult(true), // Ctrl+U clear
|
||||
sendResult(true) // image paste — into term-1
|
||||
])
|
||||
const baseSend = vi.fn().mockResolvedValue(true)
|
||||
const baseSend = vi.fn().mockResolvedValue('accepted')
|
||||
const showToast = vi.fn()
|
||||
const activeHandleRef = { current: 'term-1' }
|
||||
let releaseSettle: (() => void) | null = null
|
||||
|
|
@ -483,7 +483,7 @@ describe('useMobileNativeChatImageAttachments', () => {
|
|||
sendResult(false), // image paste rejected — stale input left in term-1
|
||||
sendResult(true) // healing Ctrl+U before the text-only send
|
||||
])
|
||||
const baseSend = vi.fn().mockResolvedValue(true)
|
||||
const baseSend = vi.fn().mockResolvedValue('accepted')
|
||||
mount(baseArgs({ client: client as unknown as RpcClient, baseSend }))
|
||||
await act(async () => {
|
||||
await hook!.attachImage('library')
|
||||
|
|
@ -510,6 +510,62 @@ describe('useMobileNativeChatImageAttachments', () => {
|
|||
expect(baseSend).toHaveBeenCalledWith('hi again')
|
||||
})
|
||||
|
||||
it('heals before the next text-only send when an image submit delivery is unknown (#10228)', async () => {
|
||||
pick.mockResolvedValue({ base64: 'AAAA', uri: 'file:///a.jpg' })
|
||||
const client = makeClient([
|
||||
methodNotFound('start'),
|
||||
ok('save', '/tmp/a.png'),
|
||||
sendResult(true), // Ctrl+U clear
|
||||
sendResult(true), // image paste accepted — path now sits on term-1's input
|
||||
sendResult(true) // healing Ctrl+U before the follow-up text send
|
||||
])
|
||||
const baseSend = vi.fn().mockResolvedValueOnce('unknown').mockResolvedValueOnce('accepted')
|
||||
mount(baseArgs({ client: client as unknown as RpcClient, baseSend }))
|
||||
await act(async () => {
|
||||
await hook!.attachImage('library')
|
||||
})
|
||||
|
||||
// Ambiguous delivery: the paste landed but the text+Enter may not have.
|
||||
let accepted = false
|
||||
await act(async () => {
|
||||
accepted = await hook!.sendNativeChat('pic')
|
||||
})
|
||||
// Mirrors the text path: 'unknown' usually WAS delivered, so the send is not
|
||||
// surfaced as a failure and the chip does not linger for a double-send retry.
|
||||
expect(accepted).toBe(true)
|
||||
expect(hook!.attachments).toEqual([])
|
||||
|
||||
// The next plain-text send must Ctrl+U first — if the Enter was lost, the
|
||||
// orphaned image path would otherwise glue onto this later message.
|
||||
await act(async () => {
|
||||
accepted = await hook!.sendNativeChat('later message')
|
||||
})
|
||||
expect(accepted).toBe(true)
|
||||
const sendCalls = client.calls.filter((c) => c.method === 'terminal.send')
|
||||
expect(sendCalls).toHaveLength(3)
|
||||
expect(sendCalls[2]?.params).toMatchObject({ text: '\x15', enter: false })
|
||||
expect(baseSend).toHaveBeenNthCalledWith(1, 'pic', ['file:///a.jpg'])
|
||||
expect(baseSend).toHaveBeenNthCalledWith(2, 'later message')
|
||||
})
|
||||
|
||||
it('does not heal after an unknown text-only send (nothing was pasted first)', async () => {
|
||||
const client = makeClient([])
|
||||
const baseSend = vi.fn().mockResolvedValueOnce('unknown').mockResolvedValueOnce('accepted')
|
||||
mount(baseArgs({ client: client as unknown as RpcClient, baseSend }))
|
||||
|
||||
let accepted = false
|
||||
await act(async () => {
|
||||
accepted = await hook!.sendNativeChat('first')
|
||||
})
|
||||
expect(accepted).toBe(true)
|
||||
await act(async () => {
|
||||
accepted = await hook!.sendNativeChat('second')
|
||||
})
|
||||
expect(accepted).toBe(true)
|
||||
// No paste preceded the ambiguous send, so no healing Ctrl+U hits the wire.
|
||||
expect(client.calls).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('retains the stale marker when a rejected healing clear blocks text-only send', async () => {
|
||||
pick.mockResolvedValue({ base64: 'AAAA', uri: 'file:///a.jpg' })
|
||||
const client = makeClient([
|
||||
|
|
@ -520,7 +576,7 @@ describe('useMobileNativeChatImageAttachments', () => {
|
|||
sendResult(false), // first healing Ctrl+U rejected
|
||||
sendResult(true) // retry healing Ctrl+U accepted
|
||||
])
|
||||
const baseSend = vi.fn().mockResolvedValueOnce(false).mockResolvedValueOnce(true)
|
||||
const baseSend = vi.fn().mockResolvedValueOnce('rejected').mockResolvedValueOnce('accepted')
|
||||
mount(baseArgs({ client: client as unknown as RpcClient, baseSend }))
|
||||
await act(async () => {
|
||||
await hook!.attachImage('library')
|
||||
|
|
@ -565,7 +621,7 @@ describe('useMobileNativeChatImageAttachments', () => {
|
|||
sendResult(true),
|
||||
deferredClear
|
||||
])
|
||||
const baseSend = vi.fn().mockResolvedValueOnce(false)
|
||||
const baseSend = vi.fn().mockResolvedValueOnce('rejected')
|
||||
const activeHandleRef = { current: 'term-1' }
|
||||
mount(baseArgs({ client: client as unknown as RpcClient, activeHandleRef, baseSend }))
|
||||
await act(async () => {
|
||||
|
|
@ -614,10 +670,10 @@ describe('useMobileNativeChatImageAttachments', () => {
|
|||
])
|
||||
const baseSend = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce(false)
|
||||
.mockResolvedValueOnce(false)
|
||||
.mockResolvedValueOnce(true)
|
||||
.mockResolvedValueOnce(true)
|
||||
.mockResolvedValueOnce('rejected')
|
||||
.mockResolvedValueOnce('rejected')
|
||||
.mockResolvedValueOnce('accepted')
|
||||
.mockResolvedValueOnce('accepted')
|
||||
const activeHandleRef = { current: 'term-1' }
|
||||
const args = baseArgs({ client: client as unknown as RpcClient, activeHandleRef, baseSend })
|
||||
mount(args)
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@ import {
|
|||
MOBILE_NATIVE_CHAT_IMAGE_SETTLE_MS,
|
||||
pasteMobileNativeChatImagePaths
|
||||
} from './mobile-native-chat-image-send'
|
||||
import type { MobileNativeChatSendOutcome } from './mobile-native-chat-send'
|
||||
|
||||
type CurrentRef<T> = { readonly current: T }
|
||||
type ShowToast = (message: string, durationMs?: number) => void
|
||||
|
|
@ -32,9 +33,14 @@ type Args = {
|
|||
/** The native-chat input lease is ready — same gate `handleNativeChatSend` uses. */
|
||||
readonly enabled: boolean
|
||||
readonly showToast: ShowToast
|
||||
/** The plain text send (controller.handleNativeChatSend); wrapped so images ride
|
||||
* along. The optional URIs drive the optimistic echo's thumbnails. */
|
||||
readonly baseSend: (text: string, imagePreviewUris?: string[]) => Promise<boolean>
|
||||
/** The plain text send (controller.handleNativeChatSendWithOutcome); wrapped so
|
||||
* images ride along. The optional URIs drive the optimistic echo's thumbnails.
|
||||
* Must preserve 'unknown': after a successful paste, an ambiguously-delivered
|
||||
* text+Enter may have left the image on the input line, which needs healing. */
|
||||
readonly baseSend: (
|
||||
text: string,
|
||||
imagePreviewUris?: string[]
|
||||
) => Promise<MobileNativeChatSendOutcome>
|
||||
readonly onAttachSuccess?: () => void
|
||||
readonly onError?: () => void
|
||||
// Injected so the settle between image paste and submit is instant in tests.
|
||||
|
|
@ -238,7 +244,8 @@ export function useMobileNativeChatImageAttachments({
|
|||
return false
|
||||
}
|
||||
}
|
||||
return baseSend(text)
|
||||
// Text-only sends paste nothing first, so 'unknown' leaves no stale input.
|
||||
return (await baseSend(text)) !== 'rejected'
|
||||
}
|
||||
const handle = activeHandleRef.current
|
||||
if (!client || !handle || !enabled || connState !== 'connected') {
|
||||
|
|
@ -276,17 +283,20 @@ export function useMobileNativeChatImageAttachments({
|
|||
showToast('Message not sent', 1500)
|
||||
return false
|
||||
}
|
||||
const accepted = await baseSend(
|
||||
const outcome = await baseSend(
|
||||
text,
|
||||
pendingImages.map((attachment) => attachment.previewUri)
|
||||
)
|
||||
if (!accepted) {
|
||||
// A rejected submit leaves the successfully pasted image path on this input line.
|
||||
if (outcome !== 'accepted') {
|
||||
// 'rejected' leaves the pasted image path on this input line; 'unknown'
|
||||
// may have lost the text+Enter AFTER the paste landed, orphaning the
|
||||
// image onto whatever is sent next (#10228) — both must heal first.
|
||||
markTerminalInputStale(staleInputTerminalsRef.current, handle)
|
||||
}
|
||||
if (accepted) {
|
||||
if (outcome !== 'rejected') {
|
||||
// Drop only what rode along — a chip attached while this send was in
|
||||
// flight keeps waiting for its own send.
|
||||
// flight keeps waiting for its own send. 'unknown' clears too: the
|
||||
// send usually DID land, and a kept chip would double-send the image.
|
||||
const sentIds = new Set(pendingImages.map((attachment) => attachment.id))
|
||||
setAttachmentsByScope((prev) =>
|
||||
withScopeAttachments(
|
||||
|
|
@ -296,7 +306,7 @@ export function useMobileNativeChatImageAttachments({
|
|||
)
|
||||
)
|
||||
}
|
||||
return accepted
|
||||
return outcome !== 'rejected'
|
||||
} catch {
|
||||
// A thrown paste/send (network/RPC) keeps the chips and honors the
|
||||
// Promise<boolean> contract instead of rejecting. Retry-safe: the next
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import type { RpcClient } from '../transport/rpc-client'
|
||||
import type { ConnectionState } from '../transport/types'
|
||||
import type { MobileImageSource } from './mobile-image-source-picker'
|
||||
import type { MobileNativeChatSendOutcome } from './mobile-native-chat-send'
|
||||
import { useMobileImageAttachment } from './use-mobile-image-attachment'
|
||||
import {
|
||||
useMobileNativeChatImageAttachments,
|
||||
|
|
@ -22,7 +23,12 @@ type Args = {
|
|||
readonly nativeChatInputLeaseReady: boolean
|
||||
readonly getActiveWorktreeConnectionId: () => Promise<string | null>
|
||||
readonly beforeTerminalSend: (terminal: string) => Promise<boolean>
|
||||
readonly nativeChatBaseSend: (text: string, images?: string[]) => Promise<boolean>
|
||||
/** Outcome-preserving so an ambiguous ('unknown') delivery after an image
|
||||
* paste can mark the terminal input for healing (#10228). */
|
||||
readonly nativeChatBaseSend: (
|
||||
text: string,
|
||||
images?: string[]
|
||||
) => Promise<MobileNativeChatSendOutcome>
|
||||
readonly showToast: (message: string, durationMs?: number) => void
|
||||
readonly onSuccess: () => void
|
||||
readonly onError: () => void
|
||||
|
|
|
|||
Loading…
Reference in New Issue