From a872d0fe6600a7b1dd33f969579693b2ea046d4c Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 4 Aug 2026 02:00:01 -0700 Subject: [PATCH] fix(mobile): report a partly-sent ask answer honestly An ask-answer chain that was aborted mid-sequence (session swap, input lease loss, Stop, a superseding answer) returned silently even when earlier keystroke groups had already been accepted by the host. The card was left looking cleanly sent while the remote selector sat half-stepped, so a retry replayed a from-scratch key plan onto an advanced position. Route the abort paths through the existing fail() reporter and let it speak when the chain wrote something, not only when it is still the current generation: an accepted group reports "Answer partly sent", an ambiguous one "Answer unconfirmed". A chain that never got a keystroke onto the wire stays silent, so a superseded answer the user replaced on purpose does not raise a banner. Refs STA-3333. --- ...use-mobile-native-chat-answer-send.test.ts | 124 +++++++++++++++++- .../use-mobile-native-chat-answer-send.ts | 32 +++-- 2 files changed, 143 insertions(+), 13 deletions(-) diff --git a/mobile/src/session/use-mobile-native-chat-answer-send.test.ts b/mobile/src/session/use-mobile-native-chat-answer-send.test.ts index 8f22884145e..d45b88a3ecc 100644 --- a/mobile/src/session/use-mobile-native-chat-answer-send.test.ts +++ b/mobile/src/session/use-mobile-native-chat-answer-send.test.ts @@ -375,8 +375,9 @@ describe('useMobileNativeChatAnswerSend', () => { }) it('cancels delayed keystrokes when the acknowledged input lease is lost', async () => { + const onSendError = vi.fn() const sendRequest = vi.fn().mockResolvedValue(acceptedResponse()) - await mount({ sendRequest } as unknown as RpcClient, vi.fn()) + await mount({ sendRequest } as unknown as RpcClient, onSendError) const prompt: AskPrompt = { questions: [ @@ -395,5 +396,126 @@ describe('useMobileNativeChatAnswerSend', () => { await expect(result).resolves.toBe(false) expect(sendRequest).toHaveBeenCalledTimes(1) + // q1's digit DID land before the lease loss aborted the chain, so the remote + // selector sits on q2 — ending silently left the card looking cleanly sent. + expect(onSendError).toHaveBeenCalledWith('Answer partly sent — check chat before retrying') + }) + + it('reports the half-stepped selector when Stop aborts a paced answer', async () => { + const onSendError = vi.fn() + const sendRequest = vi.fn().mockResolvedValue(acceptedResponse()) + await mount({ sendRequest } as unknown as RpcClient, onSendError) + + const prompt: AskPrompt = { + questions: [ + { question: 'q1', multiSelect: false, options: [{ label: 'A' }, { label: 'B' }] }, + { question: 'q2', multiSelect: false, options: [{ label: 'C' }, { label: 'D' }] } + ] + } + let result: Promise | undefined + await act(async () => { + result = answerSend?.answerAsk(prompt, [{ indices: [1] }, { indices: [0] }]) + }) + expect(sendRequest).toHaveBeenCalledTimes(1) + + await act(async () => { + answerSend?.cancelPending() + await vi.runAllTimersAsync() + }) + + await expect(result).resolves.toBe(false) + expect(sendRequest).toHaveBeenCalledTimes(1) + expect(onSendError).toHaveBeenCalledWith('Answer partly sent — check chat before retrying') + }) + + it('reports a superseded chain whose first group already moved the selector', async () => { + const onSendError = vi.fn() + const sendRequest = vi.fn().mockResolvedValue(acceptedResponse()) + await mount({ sendRequest } as unknown as RpcClient, onSendError) + + const prompt: AskPrompt = { + questions: [ + { question: 'q1', multiSelect: false, options: [{ label: 'A' }, { label: 'B' }] }, + { question: 'q2', multiSelect: false, options: [{ label: 'C' }, { label: 'D' }] } + ] + } + let first: Promise | undefined + await act(async () => { + first = answerSend?.answerAsk(prompt, [{ indices: [1] }, { indices: [0] }]) + }) + expect(sendRequest).toHaveBeenCalledTimes(1) + + // Changing the answer mid-sequence supersedes the chain, but q1's digit is + // already committed on the host — the replacement starts from a moved selector. + await act(async () => { + void answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [1] }]) + await vi.runAllTimersAsync() + }) + + await expect(first).resolves.toBe(false) + expect(onSendError).toHaveBeenCalledWith('Answer partly sent — check chat before retrying') + }) + + it('keeps an ambiguous write ambiguous when the chain is aborted after it', async () => { + const onSendError = vi.fn() + let rejectWrite: (error: Error) => void = () => undefined + const sendRequest = vi.fn( + () => + new Promise((_resolve, reject) => { + rejectWrite = reject + }) + ) + await mount({ sendRequest } as unknown as RpcClient, onSendError) + + let result: Promise | undefined + await act(async () => { + result = answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [1] }]) + await Promise.resolve() + }) + expect(sendRequest).toHaveBeenCalledTimes(1) + + await act(async () => { + answerSend?.cancelPending() + rejectWrite(markRpcDeliveryUnknown(new Error('Connection closed'))) + await vi.runAllTimersAsync() + }) + + await expect(result).resolves.toBe(false) + // The abort says nothing about whether the in-flight keystroke landed. + expect(onSendError).toHaveBeenCalledWith('Answer unconfirmed — check chat before retrying') + }) + + it('stays silent when an aborted chain never got a keystroke onto the wire', async () => { + const onSendError = vi.fn() + let settleWrite: (response: unknown) => void = () => undefined + const sendRequest = vi.fn( + () => + new Promise((resolve) => { + settleWrite = resolve + }) + ) + await mount({ sendRequest } as unknown as RpcClient, onSendError) + + let result: Promise | undefined + await act(async () => { + result = answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [1] }]) + await Promise.resolve() + }) + expect(sendRequest).toHaveBeenCalledTimes(1) + + await act(async () => { + answerSend?.cancelPending() + settleWrite({ + id: 'send', + ok: true as const, + result: { send: { accepted: false } }, + _meta: { runtimeId: 'runtime' } + }) + await vi.runAllTimersAsync() + }) + + await expect(result).resolves.toBe(false) + // Nothing reached the PTY and the user already moved on — no banner to raise. + expect(onSendError).not.toHaveBeenCalled() }) }) diff --git a/mobile/src/session/use-mobile-native-chat-answer-send.ts b/mobile/src/session/use-mobile-native-chat-answer-send.ts index 11a3a030788..33635105b09 100644 --- a/mobile/src/session/use-mobile-native-chat-answer-send.ts +++ b/mobile/src/session/use-mobile-native-chat-answer-send.ts @@ -142,8 +142,12 @@ export function useMobileNativeChatAnswerSend(args: { } return outcome === 'accepted' } - const wait = (ms: number): Promise => - new Promise((resolve) => { + const wait = (ms: number): Promise => { + // Already superseded: schedule nothing, so the report is immediate. + if (generationRef.current !== generation) { + return Promise.resolve(false) + } + return new Promise((resolve) => { const delay = { timer: setTimeout(() => { delaysRef.current.delete(delay) @@ -153,14 +157,18 @@ export function useMobileNativeChatAnswerSend(args: { } delaysRef.current.add(delay) }) + } const fail = (): false => { - if (generationRef.current === generation) { - // Why: keystrokes that may have landed (ack lost / path cutover) must - // not read as a definite failure — a blind resend could double-step - // the selector. An earlier group that WAS accepted is the same hazard - // in definite form: a multi-question answer whose shared budget ran out - // mid-sequence left the remote selector half-stepped, and telling the - // user nothing was sent invites a retry on top of the advanced state. + // Why: keystrokes that may have landed (ack lost / path cutover) must + // not read as a definite failure — a blind resend could double-step + // the selector. An earlier group that WAS accepted is the same hazard + // in definite form: a multi-question answer whose shared budget ran out + // mid-sequence left the remote selector half-stepped, and telling the + // user nothing was sent invites a retry on top of the advanced state. + // A superseded/aborted chain (session swap, lease loss, Stop, newer + // answer) stays silent only while nothing was written yet: once a group + // landed, ending silently leaves the card looking cleanly sent. + if (generationRef.current === generation || sawAcceptedGroup || sawUnknownOutcome) { onSendError( sawAcceptedGroup ? 'Answer partly sent — check chat before retrying' @@ -196,7 +204,7 @@ export function useMobileNativeChatAnswerSend(args: { return false } if (generationRef.current !== generation) { - return false + return fail() } return (await sendTerminal(formatAskAnswer(prompt, selections), true)) || fail() } @@ -206,7 +214,7 @@ export function useMobileNativeChatAnswerSend(args: { : buildAskAnswerKeys(prompt, selections) for (let index = 0; index < groups.length; index += 1) { if (generationRef.current !== generation) { - return false + return fail() } const group = groups[index]! const body = 'raw' in group ? group.raw : sanitizeAskFreeText(group.text) @@ -215,7 +223,7 @@ export function useMobileNativeChatAnswerSend(args: { } if (index < groups.length - 1) { if (!(await wait(MOBILE_NATIVE_CHAT_QUESTION_STEP_MS))) { - return false + return fail() } // Pacing is deliberate, not transport latency — don't charge it to the budget. deadline += MOBILE_NATIVE_CHAT_QUESTION_STEP_MS