diff --git a/mobile/src/session/mobile-native-chat-pending-baseline.test.ts b/mobile/src/session/mobile-native-chat-pending-baseline.test.ts index f331f9b1762..f421cbd388c 100644 --- a/mobile/src/session/mobile-native-chat-pending-baseline.test.ts +++ b/mobile/src/session/mobile-native-chat-pending-baseline.test.ts @@ -41,15 +41,16 @@ describe('rebaseMobileNativeChatPendingBaselines', () => { expect(rebaseMobileNativeChatPendingBaselines(history, pending)).toBe(pending) }) - it('pins an unresolved send to the loaded tail and past the rows it never saw', () => { + it('gives an unresolved send the loaded tail as its glue boundary', () => { expect( rebaseMobileNativeChatPendingBaselines(history, [unresolved('p1', 'run the tests')]) ).toEqual([ { id: 'p1', text: 'run the tests', - // The one row already in history is not this send's echo; it waits for the second. - expectedOccurrence: 2, + // NOT recounted against this read: the read may already carry this send's + // own echo, and counting it as history would strand the bubble forever. + expectedOccurrence: 1, baselineTailMessageId: 'm2', baselineResolved: true } @@ -68,23 +69,16 @@ describe('rebaseMobileNativeChatPendingBaselines', () => { ]) }) - it('gives identical queued sends consecutive ordinals', () => { + it('keeps the ordinals the queue already assigned', () => { expect( rebaseMobileNativeChatPendingBaselines(history, [ - unresolved('p1', 'run the tests'), - unresolved('p2', 'run the tests') + unresolved('p1', 'run the tests', 1), + unresolved('p2', 'run the tests', 2) ]).map((item) => item.expectedOccurrence) - ).toEqual([2, 3]) + ).toEqual([1, 2]) }) - it('normalizes whitespace when counting a send against the loaded rows', () => { - expect( - rebaseMobileNativeChatPendingBaselines(history, [unresolved('p1', ' run the tests \n')])[0] - ?.expectedOccurrence - ).toBe(2) - }) - - it('leaves already-resolved neighbours untouched but still counts them', () => { + it('leaves already-resolved neighbours untouched', () => { const resolved: MobileNativeChatPendingMessage = { id: 'p1', text: 'run the tests', @@ -94,19 +88,28 @@ describe('rebaseMobileNativeChatPendingBaselines', () => { } const rebased = rebaseMobileNativeChatPendingBaselines(history, [ resolved, - unresolved('p2', 'run the tests') + unresolved('p2', 'run the tests', 2) ]) expect(rebased[0]).toBe(resolved) - expect(rebased[1]?.expectedOccurrence).toBe(3) + expect(rebased[1]).toEqual({ + id: 'p2', + text: 'run the tests', + expectedOccurrence: 2, + baselineTailMessageId: 'm2', + baselineResolved: true + }) }) - it('ordinals caption-less image echoes among themselves, not against text rows', () => { + // An image echo counts image turns AFTER its tail, so moving the tail onto this + // read would discard an echo the read already carries. + it('keeps a caption-less image echo on its captured tail', () => { const images = { ...unresolved('p1', ''), images: ['file:///a.png'] } const rebased = rebaseMobileNativeChatPendingBaselines(history, [ images, - { ...unresolved('p2', ''), images: ['file:///b.png'] } + { ...unresolved('p2', '', 2), images: ['file:///b.png'] } ]) expect(rebased.map((item) => item.expectedOccurrence)).toEqual([1, 2]) - expect(rebased.map((item) => item.baselineTailMessageId)).toEqual(['m2', 'm2']) + expect(rebased.map((item) => item.baselineTailMessageId)).toEqual([null, null]) + expect(rebased.every((item) => item.baselineResolved)).toBe(true) }) }) diff --git a/mobile/src/session/mobile-native-chat-pending-baseline.ts b/mobile/src/session/mobile-native-chat-pending-baseline.ts index d381d9e60a0..995e3a6a650 100644 --- a/mobile/src/session/mobile-native-chat-pending-baseline.ts +++ b/mobile/src/session/mobile-native-chat-pending-baseline.ts @@ -1,27 +1,29 @@ import type { NativeChatMessage } from '../../../src/shared/native-chat-types' -import { - countUserTextOccurrences, - normalizeReconcileText -} from './mobile-native-chat-draft-reconcile' +import { normalizeReconcileText } from './mobile-native-chat-draft-reconcile' import type { MobileNativeChatPendingMessage } from './mobile-native-chat-pending-echo' /** - * Pin the sends issued while the transcript was still hydrating onto the first - * authoritative read of this session's history. + * Give the sends issued before this session's history was known the boundary + * they never had, on the first authoritative read. * - * Such a send has no usable baseline at capture time: `messages` was empty, or - * still the previously active tab's, so both its tail and its occurrence count - * describe somebody else's transcript. Rebasing them here — rather than marking - * the send permanently untrustworthy, which stranded it as a queued bubble and - * as a glue barrier for its neighbours for the rest of the session — leaves it - * matching exactly the rows that arrive from now on. + * Such a send saw no transcript, so it has no row it can judge candidates + * against — which held it out of matching entirely, stranding it as a queued + * bubble and, worse, as a permanent glue barrier for its neighbours. Recovering + * the tail is all that is needed: the ordinal was already counted against an + * empty transcript, which is exactly right for "no history was known". * - * Residual: if this read was taken AFTER the send already echoed — a re-subscribe - * on tab switch or reconnect, when the first load never completed — its own echo - * is counted as history and the bubble waits for a row that will not come. The - * transport writes keystrokes into a TUI, so there is no client message id to - * tell the two apart; the send is bounded instead by serializing submits per PTY - * so the glue never forms. + * The ordinal is deliberately NOT recounted against this read. The read can + * already carry the send's own echo — a re-subscribe after a tab switch or a + * reconnect returns whatever exists now — and nothing local separates that echo + * from an identical older prompt: the transport writes keystrokes into a TUI and + * carries no message id, and row timestamps come from the host while the send + * time comes from the phone. Counting it as history puts the ordinal one past + * anything the transcript can supply, which strands the bubble for the rest of + * the session and stops its whole run from ever gluing. + * + * A caption-less image echo keeps its captured tail: it reconciles by counting + * image turns AFTER that tail, so pinning the tail past this read would throw + * away an echo the read already carries. */ export function rebaseMobileNativeChatPendingBaselines( messages: readonly NativeChatMessage[], @@ -31,29 +33,12 @@ export function rebaseMobileNativeChatPendingBaselines( return current } const baselineTailMessageId = messages.at(-1)?.id ?? null - const earlierByText = new Map() - let earlierImageEchoes = 0 return current.map((item) => { - const normalized = normalizeReconcileText(item.text) - const earlier = normalized === '' ? earlierImageEchoes : (earlierByText.get(normalized) ?? 0) - if (normalized === '') { - earlierImageEchoes += item.images?.length ? 1 : 0 - } else { - earlierByText.set(normalized, earlier + 1) - } if (item.baselineResolved) { return item } - return { - ...item, - baselineTailMessageId, - baselineResolved: true, - // Ordinals stay relative to the queue: earlier still-pending sends of the - // same text claim the earlier rows, so this one waits for its own. - expectedOccurrence: - normalized === '' - ? earlier + 1 - : countUserTextOccurrences(messages, normalized) + earlier + 1 - } + return item.images?.length || normalizeReconcileText(item.text) === '' + ? { ...item, baselineResolved: true } + : { ...item, baselineResolved: true, baselineTailMessageId } }) } diff --git a/mobile/src/session/use-mobile-native-chat-controller.ts b/mobile/src/session/use-mobile-native-chat-controller.ts index 46012dadbe4..27ad8049c1d 100644 --- a/mobile/src/session/use-mobile-native-chat-controller.ts +++ b/mobile/src/session/use-mobile-native-chat-controller.ts @@ -111,7 +111,8 @@ export function useMobileNativeChatController(args: { // a null is indistinguishable from a host retraction, and peeking at the // terminal view would permanently decline the prefill. chatActive: showNativeChat, - transcriptLoading: nativeChatSession.transcriptLoading + transcriptLoading: nativeChatSession.transcriptLoading, + transcriptSettled: nativeChatSession.status === 'ready' }) const nativeChatStatus = activeChatResolution ? activeSessionTab?.agentStatus : null diff --git a/mobile/src/session/use-mobile-native-chat-drafts-glued-pending.test.ts b/mobile/src/session/use-mobile-native-chat-drafts-glued-pending.test.ts index d0df3f0faaa..7a1cf46cbe7 100644 --- a/mobile/src/session/use-mobile-native-chat-drafts-glued-pending.test.ts +++ b/mobile/src/session/use-mobile-native-chat-drafts-glued-pending.test.ts @@ -52,10 +52,12 @@ describe('useMobileNativeChatDrafts glued pending sends', () => { function Harness({ messages, - transcriptLoading = false + transcriptLoading = false, + transcriptSettled = !transcriptLoading }: { messages: NativeChatMessage[] transcriptLoading?: boolean + transcriptSettled?: boolean }): null { state = useMobileNativeChatDrafts({ hostId: 'host', @@ -63,19 +65,30 @@ describe('useMobileNativeChatDrafts glued pending sends', () => { tabId: 'tab', sessionId: 'session', messages, - transcriptLoading + transcriptLoading, + transcriptSettled }) return null } - async function mount(messages: NativeChatMessage[], transcriptLoading = false): Promise { + async function mount( + messages: NativeChatMessage[], + transcriptLoading = false, + transcriptSettled = !transcriptLoading + ): Promise { await act(async () => { - renderer = create(createElement(Harness, { messages, transcriptLoading })) + renderer = create(createElement(Harness, { messages, transcriptLoading, transcriptSettled })) }) } - async function update(messages: NativeChatMessage[], transcriptLoading = false): Promise { - await act(async () => renderer?.update(createElement(Harness, { messages, transcriptLoading }))) + async function update( + messages: NativeChatMessage[], + transcriptLoading = false, + transcriptSettled = !transcriptLoading + ): Promise { + await act(async () => + renderer?.update(createElement(Harness, { messages, transcriptLoading, transcriptSettled })) + ) } function send(text: string): void { @@ -213,17 +226,71 @@ describe('useMobileNativeChatDrafts glued pending sends', () => { expect(state?.pending).toEqual([]) }) - it('does not retire a hydration-time send against history it never saw', async () => { + // The read that resolves a hydration-time send can already contain that + // send's own echo — a re-subscribe returns whatever exists now — and nothing + // local tells that echo from an identical older prompt. Claiming the row is + // the safe direction: being wrong costs one round trip of invisibility, + // while holding costs a queued bubble that never clears and a run that can + // never glue again. + it('retires a hydration-time send whose echo arrives with the read', async () => { await mount([], true) act(() => send('run the tests')) - // The read lands carrying an older identical prompt: not this send's echo. + const echoed = [userTurn('m1', 'run the tests', 1000), assistantTurn('m2', 'passed', 1100)] + await update(echoed) + expect(state?.pending).toEqual([]) + }) + + // The same read seen one reconnect later: the send left, the agent echoed it, + // and only then did the first authoritative read land. + it('retires a hydration-time send after a reconnect delivered the read late', async () => { + await mount([], true) + act(() => send('run the tests')) + // Still loading: the first read never arrived before the drop. + await update([], true) + + const afterReconnect = [ + userTurn('m1', 'run the tests', 5000), + assistantTurn('m2', 'ok', 5100) + ] + await update(afterReconnect) + expect(state?.pending).toEqual([]) + + // Nothing later resurrects it, and the run stays glue-capable. + await update([...afterReconnect, assistantTurn('m3', 'all green', 6000)]) + expect(state?.pending).toEqual([]) + }) + }) + + // A read that failed is not a boundary: `messages` is empty because the history + // is unknown, not because the conversation is. Without that distinction the + // empty list reads as "the transcript was empty", and any row the successful + // read finally brings can claim these sends. + describe('sends issued before any read settled', () => { + it('keeps both bubbles when the late read carries only an older glue-alike', async () => { + await mount([], false, false) + rapidSend('fix the', 'bug') + const olderIdentical = [ - userTurn('m1', 'run the tests', 1000), - assistantTurn('m2', 'passed', 1100) + userTurn('m1', 'fix the bug', 1000), + assistantTurn('m2', 'fixed', 1100) ] await update(olderIdentical) - expect(pendingTexts()).toEqual(['run the tests']) + expect(pendingTexts()).toEqual(['fix the', 'bug']) + expect(renderedPendingTexts(olderIdentical, state)).toEqual(['fix the', 'bug']) + }) + + it('still retires them once their own glued row lands', async () => { + await mount([], false, false) + rapidSend('fix the', 'bug') + const olderIdentical = [ + userTurn('m1', 'fix the bug', 1000), + assistantTurn('m2', 'fixed', 1100) + ] + await update(olderIdentical) + + await update([...olderIdentical, userTurn('m3', 'fix the bug', 5000)]) + expect(state?.pending).toEqual([]) }) }) }) diff --git a/mobile/src/session/use-mobile-native-chat-drafts.ts b/mobile/src/session/use-mobile-native-chat-drafts.ts index 2a68bb6bd06..64cdf7cdc99 100644 --- a/mobile/src/session/use-mobile-native-chat-drafts.ts +++ b/mobile/src/session/use-mobile-native-chat-drafts.ts @@ -47,6 +47,10 @@ export function useMobileNativeChatDrafts(args: { * transcript still belongs to the previously active tab), so it cannot be * trusted to decline or retire the seed. */ transcriptLoading?: boolean + /** `messages` is this session's own settled history — so an empty one really + * is an empty conversation, not a read that failed or never ran. Only then + * does a send's captured tail describe a real boundary. */ + transcriptSettled?: boolean }): { composerText: string setComposerText: Dispatch> @@ -80,7 +84,8 @@ export function useMobileNativeChatDrafts(args: { launchDraft, launchDraftCreatedAt, chatActive = true, - transcriptLoading + transcriptLoading, + transcriptSettled = !transcriptLoading } = args const draftKey = mobileNativeChatScopeKey(hostId, worktreeId, tabId) const pendingKey = draftKey && sessionId ? `${draftKey}\0${sessionId}` : null @@ -139,12 +144,13 @@ export function useMobileNativeChatDrafts(args: { normalizedText, baselineOccurrences: countUserTextOccurrences(messagesRef.current, normalizedText), baselineTailMessageId: messagesRef.current.at(-1)?.id ?? null, - // A hydrating transcript is not yet this session's, so this baseline is - // a placeholder the first authoritative read rebases. - baselineResolved: !transcriptLoading + // Only a settled read makes this a boundary. Anything else — hydrating, + // or a read that failed — hands back an empty list that reads as "the + // conversation was empty", which lets any row claim this send later. + baselineResolved: transcriptSettled } }, - [draftKey, pendingKey, transcriptLoading] + [draftKey, pendingKey, transcriptSettled] ) // Why: over relay the send RPC can take seconds (or lose only its ack), and a @@ -294,11 +300,11 @@ export function useMobileNativeChatDrafts(args: { } setPendingBySession((previous) => { const current = previous[pendingKey] ?? [] - // Rebase before retiring: a hydration-time send has to own a real boundary - // before any row can be judged against it. - const rebased = transcriptLoading - ? current - : rebaseMobileNativeChatPendingBaselines(messages, current) + // Rebase before retiring: a send captured before the history was known has + // to own a real boundary before any row can be judged against it. + const rebased = transcriptSettled + ? rebaseMobileNativeChatPendingBaselines(messages, current) + : current const next = retireLandedMobileNativeChatPending(messages, rebased, landedImagePendingIds) if (next === current) { return previous @@ -310,7 +316,7 @@ export function useMobileNativeChatDrafts(args: { delete remaining[pendingKey] return remaining }) - }, [messages, pending, pendingKey, transcriptLoading]) + }, [messages, pending, pendingKey, transcriptSettled]) return { composerText: draftKey ? (drafts[draftKey] ?? '') : '',