mirror of
https://github.com/stablyai/orca.git
synced 2026-10-18 16:03:17 +00:00
fix(mobile): clear native-chat composer optimistically at send time
Over relay the send RPC round trip is visible and a lost ack (or a relay/direct cutover) could strand the sent prompt in the composer forever: the unconfirmed-send deadline dropped its tracking entry, so a late transcript echo could never clear the draft. Clear the draft at send time and restore it only on a definite rejection. holdUnconfirmedSend now only manages the delivery-unconfirmed notice; it no longer touches drafts.
This commit is contained in:
@@ -110,6 +110,8 @@ export function useMobileNativeChatController(args: {
|
||||
setComposerText: setChatComposerText,
|
||||
pending: chatPending,
|
||||
captureSendOrigin,
|
||||
clearDraftForSend,
|
||||
restoreRejectedDraft,
|
||||
acceptSend,
|
||||
holdUnconfirmedSend
|
||||
} = useMobileNativeChatDrafts({
|
||||
@@ -229,6 +231,10 @@ export function useMobileNativeChatController(args: {
|
||||
onSendError('Message not sent (disconnected)')
|
||||
return false
|
||||
}
|
||||
// Why: empty the composer at send time, not on the ack — over relay the
|
||||
// round trip is visible, and a lost ack must not strand the sent prompt
|
||||
// in the box. Only a definite rejection puts the text back.
|
||||
clearDraftForSend(origin, text)
|
||||
const outcome = await sendMobileNativeChatMessageWithOutcome({
|
||||
client,
|
||||
terminal: handle,
|
||||
@@ -246,6 +252,7 @@ export function useMobileNativeChatController(args: {
|
||||
return true
|
||||
}
|
||||
if (outcome === 'rejected') {
|
||||
restoreRejectedDraft(origin, text)
|
||||
onSendError('Message not sent')
|
||||
return false
|
||||
}
|
||||
@@ -256,11 +263,13 @@ export function useMobileNativeChatController(args: {
|
||||
acceptSend,
|
||||
activeHandleRef,
|
||||
captureSendOrigin,
|
||||
clearDraftForSend,
|
||||
client,
|
||||
deviceTokenRef,
|
||||
holdUnconfirmedSend,
|
||||
nativeChatInputLeaseReady,
|
||||
onSendError
|
||||
onSendError,
|
||||
restoreRejectedDraft
|
||||
]
|
||||
)
|
||||
|
||||
|
||||
@@ -85,6 +85,11 @@ describe('useMobileNativeChatDrafts', () => {
|
||||
act(() => state?.setComposerText('from a'))
|
||||
const originA = state?.captureSendOrigin('from a')
|
||||
expect(originA).not.toBeNull()
|
||||
act(() => {
|
||||
if (originA) {
|
||||
state?.clearDraftForSend(originA, 'from a')
|
||||
}
|
||||
})
|
||||
|
||||
await switchTo('b')
|
||||
act(() => state?.setComposerText('from b'))
|
||||
@@ -101,6 +106,99 @@ describe('useMobileNativeChatDrafts', () => {
|
||||
expect(state?.pending.map((pending) => pending.text)).toEqual(['from a'])
|
||||
})
|
||||
|
||||
it('clears the composer at send time, before the RPC settles', async () => {
|
||||
await mount('a')
|
||||
act(() => state?.setComposerText('ping'))
|
||||
const origin = state?.captureSendOrigin('ping')
|
||||
act(() => {
|
||||
if (origin) {
|
||||
state?.clearDraftForSend(origin, 'ping')
|
||||
}
|
||||
})
|
||||
expect(state?.composerText).toBe('')
|
||||
})
|
||||
|
||||
it('restores the text on a definite rejection', async () => {
|
||||
await mount('a')
|
||||
act(() => state?.setComposerText('ping'))
|
||||
const origin = state?.captureSendOrigin('ping')
|
||||
act(() => {
|
||||
if (origin) {
|
||||
state?.clearDraftForSend(origin, 'ping')
|
||||
state?.restoreRejectedDraft(origin, 'ping')
|
||||
}
|
||||
})
|
||||
expect(state?.composerText).toBe('ping')
|
||||
})
|
||||
|
||||
it('does not clobber newer edits when restoring a rejected send', async () => {
|
||||
await mount('a')
|
||||
act(() => state?.setComposerText('ping'))
|
||||
const origin = state?.captureSendOrigin('ping')
|
||||
act(() => {
|
||||
if (origin) {
|
||||
state?.clearDraftForSend(origin, 'ping')
|
||||
}
|
||||
})
|
||||
act(() => state?.setComposerText('newer edit'))
|
||||
act(() => {
|
||||
if (origin) {
|
||||
state?.restoreRejectedDraft(origin, 'ping')
|
||||
}
|
||||
})
|
||||
expect(state?.composerText).toBe('newer edit')
|
||||
})
|
||||
|
||||
it('restores a rejected send onto its originating tab only', async () => {
|
||||
await mount('a')
|
||||
act(() => state?.setComposerText('from a'))
|
||||
const originA = state?.captureSendOrigin('from a')
|
||||
act(() => {
|
||||
if (originA) {
|
||||
state?.clearDraftForSend(originA, 'from a')
|
||||
}
|
||||
})
|
||||
|
||||
await switchTo('b')
|
||||
act(() => {
|
||||
if (originA) {
|
||||
state?.restoreRejectedDraft(originA, 'from a')
|
||||
}
|
||||
})
|
||||
expect(state?.composerText).toBe('')
|
||||
|
||||
await switchTo('a')
|
||||
expect(state?.composerText).toBe('from a')
|
||||
})
|
||||
|
||||
it('keeps the composer clear when the echo lands after the unconfirmed deadline', async () => {
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
await mount('a')
|
||||
act(() => state?.setComposerText('ping'))
|
||||
const origin = state?.captureSendOrigin('ping')
|
||||
act(() => {
|
||||
if (origin) {
|
||||
state?.clearDraftForSend(origin, 'ping')
|
||||
state?.holdUnconfirmedSend(origin, 'ping', vi.fn())
|
||||
}
|
||||
})
|
||||
expect(state?.composerText).toBe('')
|
||||
|
||||
// A relay drop can stall the transcript stream past the deadline; the
|
||||
// delivered prompt must not reappear in the composer when it recovers.
|
||||
act(() => vi.advanceTimersByTime(25_000))
|
||||
await act(async () =>
|
||||
renderer?.update(
|
||||
createElement(Harness, { tabId: 'a', messages: [userTextMessage('m1', 'ping')] })
|
||||
)
|
||||
)
|
||||
expect(state?.composerText).toBe('')
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
|
||||
it('clears one pending per landed message so duplicate sends are not all dropped', async () => {
|
||||
await mount('a')
|
||||
const origin = state?.captureSendOrigin('ping')
|
||||
@@ -159,25 +257,24 @@ describe('useMobileNativeChatDrafts', () => {
|
||||
expect(state?.pending).toEqual([])
|
||||
})
|
||||
|
||||
it('does not erase newer edits when an older send settles', async () => {
|
||||
it('does not erase newer edits when an older send clears', async () => {
|
||||
await mount('a')
|
||||
act(() => state?.setComposerText('submitted'))
|
||||
const origin = state?.captureSendOrigin('submitted')
|
||||
act(() => state?.setComposerText('new edit'))
|
||||
act(() => {
|
||||
if (origin) {
|
||||
state?.acceptSend(origin, 'submitted')
|
||||
state?.clearDraftForSend(origin, 'submitted')
|
||||
}
|
||||
})
|
||||
|
||||
expect(state?.composerText).toBe('new edit')
|
||||
})
|
||||
|
||||
it('clears the draft when an unconfirmed send lands in the transcript', async () => {
|
||||
it('stays quiet when an unconfirmed send lands in the transcript', async () => {
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
await mount('a')
|
||||
act(() => state?.setComposerText('ping'))
|
||||
const origin = state?.captureSendOrigin('ping')
|
||||
const onUnconfirmed = vi.fn()
|
||||
act(() => {
|
||||
@@ -185,14 +282,12 @@ describe('useMobileNativeChatDrafts', () => {
|
||||
state?.holdUnconfirmedSend(origin, 'ping', onUnconfirmed)
|
||||
}
|
||||
})
|
||||
expect(state?.composerText).toBe('ping')
|
||||
|
||||
await act(async () =>
|
||||
renderer?.update(
|
||||
createElement(Harness, { tabId: 'a', messages: [userTextMessage('m1', 'ping')] })
|
||||
)
|
||||
)
|
||||
expect(state?.composerText).toBe('')
|
||||
|
||||
act(() => vi.advanceTimersByTime(30_000))
|
||||
expect(onUnconfirmed).not.toHaveBeenCalled()
|
||||
@@ -201,11 +296,10 @@ describe('useMobileNativeChatDrafts', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('clears immediately when the transcript echo beat the ambiguous RPC rejection', async () => {
|
||||
it('registers no deadline when the transcript echo beat the ambiguous RPC rejection', async () => {
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
await mount('a')
|
||||
act(() => state?.setComposerText('ping'))
|
||||
const origin = state?.captureSendOrigin('ping')
|
||||
const onUnconfirmed = vi.fn()
|
||||
|
||||
@@ -220,7 +314,6 @@ describe('useMobileNativeChatDrafts', () => {
|
||||
}
|
||||
})
|
||||
|
||||
expect(state?.composerText).toBe('')
|
||||
expect(vi.getTimerCount()).toBe(0)
|
||||
act(() => vi.advanceTimersByTime(30_000))
|
||||
expect(onUnconfirmed).not.toHaveBeenCalled()
|
||||
@@ -229,11 +322,10 @@ describe('useMobileNativeChatDrafts', () => {
|
||||
}
|
||||
})
|
||||
|
||||
it('surfaces uncertainty and keeps the draft when no echo lands before the deadline', async () => {
|
||||
it('surfaces uncertainty when no echo lands before the deadline', async () => {
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
await mount('a')
|
||||
act(() => state?.setComposerText('ping'))
|
||||
const origin = state?.captureSendOrigin('ping')
|
||||
const onUnconfirmed = vi.fn()
|
||||
act(() => {
|
||||
@@ -246,7 +338,6 @@ describe('useMobileNativeChatDrafts', () => {
|
||||
expect(onUnconfirmed).not.toHaveBeenCalled()
|
||||
act(() => vi.advanceTimersByTime(1))
|
||||
expect(onUnconfirmed).toHaveBeenCalledTimes(1)
|
||||
expect(state?.composerText).toBe('ping')
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
@@ -433,6 +524,7 @@ describe('useMobileNativeChatDrafts', () => {
|
||||
expect(origin).toMatchObject({ pendingKey: null })
|
||||
act(() => {
|
||||
if (origin) {
|
||||
state?.clearDraftForSend(origin, 'start the session')
|
||||
state?.acceptSend(origin, 'start the session')
|
||||
}
|
||||
})
|
||||
|
||||
@@ -99,6 +99,10 @@ export function useMobileNativeChatDrafts(args: {
|
||||
setComposerText: Dispatch<SetStateAction<string>>
|
||||
pending: MobileNativeChatPendingMessage[]
|
||||
captureSendOrigin: (text: string) => MobileNativeChatSendOrigin | null
|
||||
/** Clear the composer at send time, before the RPC settles. */
|
||||
clearDraftForSend: (origin: MobileNativeChatSendOrigin, text: string) => void
|
||||
/** Put the text back after a definite rejection, unless newer edits exist. */
|
||||
restoreRejectedDraft: (origin: MobileNativeChatSendOrigin, text: string) => void
|
||||
acceptSend: (origin: MobileNativeChatSendOrigin, text: string) => void
|
||||
holdUnconfirmedSend: (
|
||||
origin: MobileNativeChatSendOrigin,
|
||||
@@ -154,16 +158,27 @@ export function useMobileNativeChatDrafts(args: {
|
||||
[draftKey, pendingKey]
|
||||
)
|
||||
|
||||
const acceptSend = useCallback((origin: MobileNativeChatSendOrigin, text: string) => {
|
||||
// Why: an RPC may settle after a tab switch; mutate only the tab that
|
||||
// originated the send, without erasing edits typed after it began.
|
||||
// Why: over relay the send RPC can take seconds (or lose only its ack), and a
|
||||
// composer that waits for settlement to empty reads as "my prompt didn't
|
||||
// send". Clear at send time; a definite rejection restores the text below.
|
||||
const clearDraftForSend = useCallback((origin: MobileNativeChatSendOrigin, text: string) => {
|
||||
setDrafts((previous) =>
|
||||
(previous[origin.draftKey] ?? '').trim() === text.trim()
|
||||
? { ...previous, [origin.draftKey]: '' }
|
||||
: previous
|
||||
)
|
||||
}, [])
|
||||
|
||||
const restoreRejectedDraft = useCallback((origin: MobileNativeChatSendOrigin, text: string) => {
|
||||
// Why: never clobber text the user typed while the rejection was in flight.
|
||||
setDrafts((previous) =>
|
||||
(previous[origin.draftKey] ?? '') === '' ? { ...previous, [origin.draftKey]: text } : previous
|
||||
)
|
||||
}, [])
|
||||
|
||||
const acceptSend = useCallback((origin: MobileNativeChatSendOrigin, text: string) => {
|
||||
// Why: the first prompt can be sent before the provider reports a session
|
||||
// id; clear its draft, but wait for an id before keying an optimistic echo.
|
||||
// id; wait for an id before keying an optimistic echo.
|
||||
if (!origin.pendingKey) {
|
||||
return
|
||||
}
|
||||
@@ -187,8 +202,9 @@ export function useMobileNativeChatDrafts(args: {
|
||||
|
||||
// Why: a relay drop mid-send loses only the ack in the common case — the
|
||||
// desktop already delivered the message. Hold the send instead of claiming
|
||||
// failure (which baits a duplicate): clear the draft when the transcript echo
|
||||
// failure (which baits a duplicate): stay quiet when the transcript echo
|
||||
// lands, and surface the uncertainty if the deadline passes without one.
|
||||
// The composer was already cleared at send time, so this never touches drafts.
|
||||
const unconfirmedRef = useRef<UnconfirmedSend[]>([])
|
||||
const holdUnconfirmedSend = useCallback(
|
||||
(origin: MobileNativeChatSendOrigin, text: string, onUnconfirmed: () => void) => {
|
||||
@@ -211,11 +227,6 @@ export function useMobileNativeChatDrafts(args: {
|
||||
isActiveTranscript &&
|
||||
findLandedUnconfirmedSends(messagesRef.current, [entry]).length > 0
|
||||
) {
|
||||
setDrafts((previous) =>
|
||||
(previous[origin.draftKey] ?? '').trim() === text.trim()
|
||||
? { ...previous, [origin.draftKey]: '' }
|
||||
: previous
|
||||
)
|
||||
return
|
||||
}
|
||||
entry.deadline = setTimeout(() => {
|
||||
@@ -246,12 +257,6 @@ export function useMobileNativeChatDrafts(args: {
|
||||
if (entry.deadline !== null) {
|
||||
clearTimeout(entry.deadline)
|
||||
}
|
||||
// Same guard as acceptSend: never erase edits typed after the send began.
|
||||
setDrafts((previous) =>
|
||||
(previous[entry.draftKey] ?? '').trim() === entry.text.trim()
|
||||
? { ...previous, [entry.draftKey]: '' }
|
||||
: previous
|
||||
)
|
||||
}
|
||||
}, [messages, draftKey, pendingKey])
|
||||
|
||||
@@ -306,6 +311,8 @@ export function useMobileNativeChatDrafts(args: {
|
||||
setComposerText,
|
||||
pending,
|
||||
captureSendOrigin,
|
||||
clearDraftForSend,
|
||||
restoreRejectedDraft,
|
||||
acceptSend,
|
||||
holdUnconfirmedSend
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user