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.
This commit is contained in:
Brennan Benson
2026-08-04 02:00:01 -07:00
parent e9cf106769
commit a872d0fe66
2 changed files with 143 additions and 13 deletions
@@ -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<boolean> | 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<boolean> | 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<boolean> | 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<boolean> | 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()
})
})
@@ -142,8 +142,12 @@ export function useMobileNativeChatAnswerSend(args: {
}
return outcome === 'accepted'
}
const wait = (ms: number): Promise<boolean> =>
new Promise((resolve) => {
const wait = (ms: number): Promise<boolean> => {
// 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