mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 00:02:31 +00:00
fix(mobile): make image send retries safe (#10228)
This commit is contained in:
Brennan Benson
@@ -31,7 +31,10 @@ function sendResult(accepted: boolean): RpcSuccess {
|
||||
return { id: 'send', ok: true, result: { send: { accepted } }, _meta: { runtimeId: 'r' } }
|
||||
}
|
||||
|
||||
function makeClient(responses: RpcResponse[]): Pick<RpcClient, 'sendRequest'> & {
|
||||
function makeClient(responses: (RpcResponse | Promise<RpcResponse>)[]): Pick<
|
||||
RpcClient,
|
||||
'sendRequest'
|
||||
> & {
|
||||
calls: { method: string; params: Record<string, unknown> }[]
|
||||
} {
|
||||
const calls: { method: string; params: Record<string, unknown> }[] = []
|
||||
@@ -52,6 +55,7 @@ type HookArgs = Parameters<typeof useMobileNativeChatImageAttachments>[0]
|
||||
type Hook = ReturnType<typeof useMobileNativeChatImageAttachments>
|
||||
|
||||
const SCOPE_A = 'h\0w\0tab-a'
|
||||
const SCOPE_B = 'h\0w\0tab-b'
|
||||
|
||||
function baseArgs(overrides: Partial<HookArgs> & Pick<HookArgs, 'client'>): HookArgs {
|
||||
return {
|
||||
@@ -403,6 +407,14 @@ describe('useMobileNativeChatImageAttachments', () => {
|
||||
await act(async () => {
|
||||
await hook!.attachImage('library')
|
||||
})
|
||||
let overlappingAccepted = true
|
||||
await act(async () => {
|
||||
overlappingAccepted = await hook!.sendNativeChat('too soon')
|
||||
})
|
||||
expect(overlappingAccepted).toBe(false)
|
||||
expect(baseSend).not.toHaveBeenCalled()
|
||||
expect(client.calls.filter((call) => call.method === 'terminal.send')).toHaveLength(2)
|
||||
|
||||
await act(async () => {
|
||||
releaseSettle!()
|
||||
await sendPromise
|
||||
@@ -498,6 +510,159 @@ describe('useMobileNativeChatImageAttachments', () => {
|
||||
expect(baseSend).toHaveBeenCalledWith('hi again')
|
||||
})
|
||||
|
||||
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([
|
||||
methodNotFound('start'),
|
||||
ok('save', '/tmp/a.png'),
|
||||
sendResult(true), // Ctrl+U clear
|
||||
sendResult(true), // image paste accepted
|
||||
sendResult(false), // first healing Ctrl+U rejected
|
||||
sendResult(true) // retry healing Ctrl+U accepted
|
||||
])
|
||||
const baseSend = vi.fn().mockResolvedValueOnce(false).mockResolvedValueOnce(true)
|
||||
mount(baseArgs({ client: client as unknown as RpcClient, baseSend }))
|
||||
await act(async () => {
|
||||
await hook!.attachImage('library')
|
||||
})
|
||||
await act(async () => {
|
||||
await hook!.sendNativeChat('hi')
|
||||
})
|
||||
expect(hook!.attachments).toHaveLength(1)
|
||||
|
||||
await act(async () => {
|
||||
hook!.removeAttachment('img-1')
|
||||
})
|
||||
let accepted = true
|
||||
await act(async () => {
|
||||
accepted = await hook!.sendNativeChat('hi again')
|
||||
})
|
||||
expect(accepted).toBe(false)
|
||||
expect(baseSend).toHaveBeenCalledTimes(1)
|
||||
|
||||
await act(async () => {
|
||||
accepted = await hook!.sendNativeChat('hi again')
|
||||
})
|
||||
expect(accepted).toBe(true)
|
||||
const sendCalls = client.calls.filter((c) => c.method === 'terminal.send')
|
||||
expect(sendCalls).toHaveLength(4)
|
||||
expect(sendCalls[2]?.params).toMatchObject({ text: '\x15', enter: false })
|
||||
expect(sendCalls[3]?.params).toMatchObject({ text: '\x15', enter: false })
|
||||
expect(baseSend).toHaveBeenNthCalledWith(1, 'hi', ['file:///a.jpg'])
|
||||
expect(baseSend).toHaveBeenNthCalledWith(2, 'hi again')
|
||||
})
|
||||
|
||||
it('does not reroute text when the active terminal changes during a healing clear', async () => {
|
||||
pick.mockResolvedValue({ base64: 'AAAA', uri: 'file:///a.jpg' })
|
||||
let releaseClear: ((response: RpcResponse) => void) | null = null
|
||||
const deferredClear = new Promise<RpcResponse>((resolve) => {
|
||||
releaseClear = resolve
|
||||
})
|
||||
const client = makeClient([
|
||||
methodNotFound('start'),
|
||||
ok('save', '/tmp/a.png'),
|
||||
sendResult(true),
|
||||
sendResult(true),
|
||||
deferredClear
|
||||
])
|
||||
const baseSend = vi.fn().mockResolvedValueOnce(false)
|
||||
const activeHandleRef = { current: 'term-1' }
|
||||
mount(baseArgs({ client: client as unknown as RpcClient, activeHandleRef, baseSend }))
|
||||
await act(async () => {
|
||||
await hook!.attachImage('library')
|
||||
})
|
||||
await act(async () => {
|
||||
await hook!.sendNativeChat('hi')
|
||||
})
|
||||
await act(async () => {
|
||||
hook!.removeAttachment('img-1')
|
||||
})
|
||||
|
||||
let retry: Promise<boolean> | null = null
|
||||
await act(async () => {
|
||||
retry = hook!.sendNativeChat('hi again')
|
||||
await Promise.resolve()
|
||||
})
|
||||
activeHandleRef.current = 'term-2'
|
||||
let accepted = true
|
||||
await act(async () => {
|
||||
releaseClear!(sendResult(true))
|
||||
accepted = await retry!
|
||||
})
|
||||
|
||||
expect(accepted).toBe(false)
|
||||
expect(baseSend).toHaveBeenCalledTimes(1)
|
||||
const sendCalls = client.calls.filter((c) => c.method === 'terminal.send')
|
||||
expect(sendCalls[2]?.params).toMatchObject({ terminal: 'term-1', text: '\x15', enter: false })
|
||||
})
|
||||
|
||||
it('heals rejected image submits independently across terminals', async () => {
|
||||
pick
|
||||
.mockResolvedValueOnce({ base64: 'AAAA', uri: 'file:///a.jpg' })
|
||||
.mockResolvedValueOnce({ base64: 'BBBB', uri: 'file:///b.jpg' })
|
||||
const client = makeClient([
|
||||
methodNotFound('start-a'),
|
||||
ok('save-a', '/tmp/a.png'),
|
||||
sendResult(true),
|
||||
sendResult(true),
|
||||
methodNotFound('start-b'),
|
||||
ok('save-b', '/tmp/b.png'),
|
||||
sendResult(true),
|
||||
sendResult(true),
|
||||
sendResult(true),
|
||||
sendResult(true)
|
||||
])
|
||||
const baseSend = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce(false)
|
||||
.mockResolvedValueOnce(false)
|
||||
.mockResolvedValueOnce(true)
|
||||
.mockResolvedValueOnce(true)
|
||||
const activeHandleRef = { current: 'term-1' }
|
||||
const args = baseArgs({ client: client as unknown as RpcClient, activeHandleRef, baseSend })
|
||||
mount(args)
|
||||
await act(async () => {
|
||||
await hook!.attachImage('library')
|
||||
})
|
||||
await act(async () => {
|
||||
await hook!.sendNativeChat('first')
|
||||
})
|
||||
|
||||
activeHandleRef.current = 'term-2'
|
||||
update({ ...args, scopeKey: SCOPE_B })
|
||||
await act(async () => {
|
||||
await hook!.attachImage('library')
|
||||
})
|
||||
await act(async () => {
|
||||
await hook!.sendNativeChat('second')
|
||||
})
|
||||
await act(async () => {
|
||||
hook!.removeAttachment('img-2')
|
||||
})
|
||||
|
||||
activeHandleRef.current = 'term-1'
|
||||
update(args)
|
||||
await act(async () => {
|
||||
hook!.removeAttachment('img-1')
|
||||
})
|
||||
await act(async () => {
|
||||
expect(await hook!.sendNativeChat('retry first')).toBe(true)
|
||||
})
|
||||
|
||||
activeHandleRef.current = 'term-2'
|
||||
update({ ...args, scopeKey: SCOPE_B })
|
||||
await act(async () => {
|
||||
expect(await hook!.sendNativeChat('retry second')).toBe(true)
|
||||
})
|
||||
|
||||
const sendCalls = client.calls.filter((c) => c.method === 'terminal.send')
|
||||
expect(sendCalls.slice(4).map((call) => call.params)).toMatchObject([
|
||||
{ terminal: 'term-1', text: '\x15', enter: false },
|
||||
{ terminal: 'term-2', text: '\x15', enter: false }
|
||||
])
|
||||
expect(baseSend).toHaveBeenCalledTimes(4)
|
||||
})
|
||||
|
||||
it('reports a disconnected attach failure via the live connection state', async () => {
|
||||
const client = makeClient([])
|
||||
const showToast = vi.fn()
|
||||
|
||||
@@ -71,6 +71,10 @@ function withScopeAttachments(
|
||||
return remaining
|
||||
}
|
||||
|
||||
function markTerminalInputStale(staleInputs: Set<string>, terminal: string): void {
|
||||
staleInputs.add(terminal)
|
||||
}
|
||||
|
||||
const defaultSleep = (ms: number): Promise<void> =>
|
||||
new Promise((resolve) => setTimeout(resolve, ms))
|
||||
|
||||
@@ -99,9 +103,10 @@ export function useMobileNativeChatImageAttachments({
|
||||
// checked 'connected' at entry, so only a ref can see a mid-upload disconnect.
|
||||
const connStateRef = useRef(connState)
|
||||
connStateRef.current = connState
|
||||
// Terminal whose input line may hold a partial paste from a failed send; the
|
||||
// next send TO THAT terminal must lead with Ctrl+U even if it has no images.
|
||||
const staleInputTerminalRef = useRef<string | null>(null)
|
||||
// Terminals whose input may hold a failed paste; each must heal independently.
|
||||
const staleInputTerminalsRef = useRef(new Set<string>())
|
||||
// Serialize clear/paste/submit ownership per terminal while allowing other tabs to send.
|
||||
const sendInFlightTerminalsRef = useRef(new Set<string>())
|
||||
|
||||
const attachments = (scopeKey ? attachmentsByScope[scopeKey] : undefined) ?? NO_ATTACHMENTS
|
||||
|
||||
@@ -192,92 +197,120 @@ export function useMobileNativeChatImageAttachments({
|
||||
|
||||
const sendNativeChat = useCallback(
|
||||
async (text: string): Promise<boolean> => {
|
||||
const scope = scopeKey
|
||||
const pendingImages = (scope ? attachmentsByScope[scope] : undefined) ?? NO_ATTACHMENTS
|
||||
if (pendingImages.length === 0 || !scope) {
|
||||
// Heal a previously failed paste: a text-only send to that terminal would
|
||||
// otherwise glue the stale image paste onto this message. Best-effort —
|
||||
// on failure the marker stays set and the send proceeds as before.
|
||||
const staleTerminal = staleInputTerminalRef.current
|
||||
if (staleTerminal && staleTerminal === activeHandleRef.current && client) {
|
||||
try {
|
||||
await pasteMobileNativeChatImagePaths({
|
||||
client,
|
||||
terminal: staleTerminal,
|
||||
deviceToken: deviceTokenRef.current,
|
||||
imagePaths: []
|
||||
})
|
||||
staleInputTerminalRef.current = null
|
||||
} catch {
|
||||
// Leave marked for the next attempt.
|
||||
}
|
||||
}
|
||||
return baseSend(text)
|
||||
}
|
||||
const handle = activeHandleRef.current
|
||||
if (!client || !handle || !enabled || connState !== 'connected') {
|
||||
onError?.()
|
||||
// Mirror the text path's failure surface (the base send is never reached).
|
||||
showToast('Message not sent (disconnected)', 1500)
|
||||
return false
|
||||
}
|
||||
try {
|
||||
const pasted = await pasteMobileNativeChatImagePaths({
|
||||
client,
|
||||
terminal: handle,
|
||||
deviceToken: deviceTokenRef.current,
|
||||
imagePaths: pendingImages.map((attachment) => attachment.path)
|
||||
})
|
||||
if (!pasted) {
|
||||
// Keep the chips so the user can retry; the failed paste never submitted.
|
||||
staleInputTerminalRef.current = handle
|
||||
onError?.()
|
||||
showToast('Message not sent', 1500)
|
||||
return false
|
||||
}
|
||||
// The paste's leading Ctrl+U cleared any earlier stale input in `handle`.
|
||||
if (staleInputTerminalRef.current === handle) {
|
||||
staleInputTerminalRef.current = null
|
||||
}
|
||||
// Let the TUI absorb the image paste before the text + Enter follow. The
|
||||
// preview URIs ride along to baseSend so the sent bubble shows the photo
|
||||
// immediately (empty text still submits a bare Enter through baseSend).
|
||||
await sleep(MOBILE_NATIVE_CHAT_IMAGE_SETTLE_MS)
|
||||
// The paste above targeted `handle`; a tab switch during the settle would
|
||||
// route the text + Enter to a different terminal than the images. Abort —
|
||||
// the chips keep their scope and a retry's Ctrl+U clears the stale paste.
|
||||
if (activeHandleRef.current !== handle) {
|
||||
staleInputTerminalRef.current = handle
|
||||
onError?.()
|
||||
showToast('Message not sent', 1500)
|
||||
return false
|
||||
}
|
||||
const accepted = await baseSend(
|
||||
text,
|
||||
pendingImages.map((attachment) => attachment.previewUri)
|
||||
)
|
||||
if (accepted) {
|
||||
// Drop only what rode along — a chip attached while this send was in
|
||||
// flight keeps waiting for its own send.
|
||||
const sentIds = new Set(pendingImages.map((attachment) => attachment.id))
|
||||
setAttachmentsByScope((prev) =>
|
||||
withScopeAttachments(
|
||||
prev,
|
||||
scope,
|
||||
(prev[scope] ?? []).filter((attachment) => !sentIds.has(attachment.id))
|
||||
)
|
||||
)
|
||||
}
|
||||
return accepted
|
||||
} catch {
|
||||
// A thrown paste/send (network/RPC) keeps the chips and honors the
|
||||
// Promise<boolean> contract instead of rejecting. Retry-safe: the next
|
||||
// attempt's leading Ctrl+U clears whatever fraction of the paste landed.
|
||||
staleInputTerminalRef.current = handle
|
||||
const operationTerminal = activeHandleRef.current
|
||||
if (operationTerminal && sendInFlightTerminalsRef.current.has(operationTerminal)) {
|
||||
onError?.()
|
||||
showToast('Message not sent', 1500)
|
||||
return false
|
||||
}
|
||||
if (operationTerminal) {
|
||||
sendInFlightTerminalsRef.current.add(operationTerminal)
|
||||
}
|
||||
try {
|
||||
const scope = scopeKey
|
||||
const pendingImages = (scope ? attachmentsByScope[scope] : undefined) ?? NO_ATTACHMENTS
|
||||
if (pendingImages.length === 0 || !scope) {
|
||||
// Heal a previously failed paste: a text-only send to that terminal would
|
||||
// otherwise glue the stale image paste onto this message. Best-effort —
|
||||
// on failure the marker stays set and the text must not be submitted.
|
||||
const staleTerminal = activeHandleRef.current
|
||||
if (staleTerminal && staleInputTerminalsRef.current.has(staleTerminal) && client) {
|
||||
let cleared = false
|
||||
try {
|
||||
cleared = await pasteMobileNativeChatImagePaths({
|
||||
client,
|
||||
terminal: staleTerminal,
|
||||
deviceToken: deviceTokenRef.current,
|
||||
imagePaths: []
|
||||
})
|
||||
} catch {
|
||||
// Leave marked for the next attempt.
|
||||
}
|
||||
if (!cleared) {
|
||||
onError?.()
|
||||
showToast('Message not sent', 1500)
|
||||
return false
|
||||
}
|
||||
staleInputTerminalsRef.current.delete(staleTerminal)
|
||||
if (activeHandleRef.current !== staleTerminal) {
|
||||
onError?.()
|
||||
showToast('Message not sent', 1500)
|
||||
return false
|
||||
}
|
||||
}
|
||||
return baseSend(text)
|
||||
}
|
||||
const handle = activeHandleRef.current
|
||||
if (!client || !handle || !enabled || connState !== 'connected') {
|
||||
onError?.()
|
||||
// Mirror the text path's failure surface (the base send is never reached).
|
||||
showToast('Message not sent (disconnected)', 1500)
|
||||
return false
|
||||
}
|
||||
try {
|
||||
const pasted = await pasteMobileNativeChatImagePaths({
|
||||
client,
|
||||
terminal: handle,
|
||||
deviceToken: deviceTokenRef.current,
|
||||
imagePaths: pendingImages.map((attachment) => attachment.path)
|
||||
})
|
||||
if (!pasted) {
|
||||
// Keep the chips so the user can retry; the failed paste never submitted.
|
||||
markTerminalInputStale(staleInputTerminalsRef.current, handle)
|
||||
onError?.()
|
||||
showToast('Message not sent', 1500)
|
||||
return false
|
||||
}
|
||||
// The paste's leading Ctrl+U cleared any earlier stale input in `handle`.
|
||||
staleInputTerminalsRef.current.delete(handle)
|
||||
// Let the TUI absorb the image paste before the text + Enter follow. The
|
||||
// preview URIs ride along to baseSend so the sent bubble shows the photo
|
||||
// immediately (empty text still submits a bare Enter through baseSend).
|
||||
await sleep(MOBILE_NATIVE_CHAT_IMAGE_SETTLE_MS)
|
||||
// The paste above targeted `handle`; a tab switch during the settle would
|
||||
// route the text + Enter to a different terminal than the images. Abort —
|
||||
// the chips keep their scope and a retry's Ctrl+U clears the stale paste.
|
||||
if (activeHandleRef.current !== handle) {
|
||||
markTerminalInputStale(staleInputTerminalsRef.current, handle)
|
||||
onError?.()
|
||||
showToast('Message not sent', 1500)
|
||||
return false
|
||||
}
|
||||
const accepted = await baseSend(
|
||||
text,
|
||||
pendingImages.map((attachment) => attachment.previewUri)
|
||||
)
|
||||
if (!accepted) {
|
||||
// A rejected submit leaves the successfully pasted image path on this input line.
|
||||
markTerminalInputStale(staleInputTerminalsRef.current, handle)
|
||||
}
|
||||
if (accepted) {
|
||||
// Drop only what rode along — a chip attached while this send was in
|
||||
// flight keeps waiting for its own send.
|
||||
const sentIds = new Set(pendingImages.map((attachment) => attachment.id))
|
||||
setAttachmentsByScope((prev) =>
|
||||
withScopeAttachments(
|
||||
prev,
|
||||
scope,
|
||||
(prev[scope] ?? []).filter((attachment) => !sentIds.has(attachment.id))
|
||||
)
|
||||
)
|
||||
}
|
||||
return accepted
|
||||
} catch {
|
||||
// A thrown paste/send (network/RPC) keeps the chips and honors the
|
||||
// Promise<boolean> contract instead of rejecting. Retry-safe: the next
|
||||
// attempt's leading Ctrl+U clears whatever fraction of the paste landed.
|
||||
markTerminalInputStale(staleInputTerminalsRef.current, handle)
|
||||
onError?.()
|
||||
showToast('Message not sent', 1500)
|
||||
return false
|
||||
}
|
||||
} finally {
|
||||
if (operationTerminal) {
|
||||
sendInFlightTerminalsRef.current.delete(operationTerminal)
|
||||
}
|
||||
}
|
||||
},
|
||||
[
|
||||
activeHandleRef,
|
||||
|
||||
Reference in New Issue
Block a user