From 14d0a16b7357fa4d754fbb8bf9970593f07735da Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 4 Aug 2026 03:35:02 -0700 Subject: [PATCH] fix(mobile): fence on the turn slot, not the generation counter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The supersession guards added in c36f2ce14e read `generationRef`, but `cancelPending()` bumps that counter from three callers with no successor chain: Stop, ask-cancel, and the lease effect that fires on every disconnect. An answer whose key had already landed then reported false — `fail()` never runs on an accepted write, so the card stayed up silent with Submit re-enabled, and the retry wrote the same key into a selector that key had already advanced. Test the turn slot instead: a successor takes it synchronously before its first await and cannot resolve ahead of this chain, so it means "a successor took over" exactly, without catching bare cancellations. --- ...use-mobile-native-chat-answer-send.test.ts | 61 +++++++++++++++++++ .../use-mobile-native-chat-answer-send.ts | 13 ++-- 2 files changed, 69 insertions(+), 5 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 184d1bdcc37..128498c0c8a 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 @@ -720,6 +720,67 @@ describe('useMobileNativeChatAnswerSend', () => { expect(onSendError).toHaveBeenCalledWith('Answer not sent — check chat before retrying') }) + it('reports a landed answer that Stop cancelled with no successor waiting', async () => { + const onSendError = vi.fn() + const settle: Array<(response: unknown) => void> = [] + const sendRequest = vi.fn().mockImplementation( + () => + new Promise((resolve) => { + settle.push(resolve) + }) + ) + await mount({ sendRequest } as unknown as RpcClient, onSendError) + + let first: Promise | undefined + await act(async () => { + first = acceptedAnswerAsk?.(TABS_OR_SPACES, [{ indices: [1] }]) + await Promise.resolve() + }) + // Stop, an ask cancel, and a dropped input lease all bump the generation with + // NO successor chain behind them. + await act(async () => { + answerSend?.cancelPending() + settle[0]!(acceptedResponse()) + await Promise.resolve() + }) + + // The key is on the PTY and nobody else owns the surface. Calling that a + // non-send leaves the card up and silent (fail() never runs on an accepted + // write), and the retry double-steps the selector this key already moved. + await expect(first).resolves.toBe(true) + expect(onAccepted).toHaveBeenCalledTimes(1) + expect(onSendError).not.toHaveBeenCalled() + }) + + it('reports a landed pasted answer that Stop cancelled with no successor', async () => { + const onSendError = vi.fn() + const settle: Array<(response: unknown) => void> = [] + const sendRequest = vi.fn().mockImplementation( + () => + new Promise((resolve) => { + settle.push(resolve) + }) + ) + await mount({ sendRequest } as unknown as RpcClient, onSendError, 'grok') + + let first: Promise | undefined + await act(async () => { + first = acceptedAnswerAsk?.(TABS_OR_SPACES, [{ indices: [1] }]) + await Promise.resolve() + }) + await act(async () => { + answerSend?.cancelPending() + settle[0]!(acceptedResponse()) + await Promise.resolve() + }) + + // The pasted shape already committed with Enter, so suppressing the success + // strands an answered card the user is invited to submit a second time. + await expect(first).resolves.toBe(true) + expect(onAccepted).toHaveBeenCalledTimes(1) + expect(onSendError).not.toHaveBeenCalled() + }) + it('fences a third answer behind an already-fenced successor, reporting once', async () => { const onSendError = vi.fn() const settle: Array<(response: unknown) => void> = [] 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 abb79a9b50a..7104ebe0578 100644 --- a/mobile/src/session/use-mobile-native-chat-answer-send.ts +++ b/mobile/src/session/use-mobile-native-chat-answer-send.ts @@ -247,10 +247,13 @@ export function useMobileNativeChatAnswerSend(args: { if (generationRef.current !== generation) { return false } - // A superseded chain must not report success either: an accepted answer - // retires the shared send-error banner, wiping the successor's fence. + // A chain a successor took over from must not report success either: an + // accepted answer retires the shared send-error banner, wiping the + // successor's fence. Test the turn slot, not the generation counter — + // Stop, ask-cancel and a dropped lease all bump the generation with no + // successor, and there a landed answer IS a success. const sent = (await sendTerminal(formatAskAnswer(prompt, selections), true)) || fail() - return sent && generationRef.current === generation + return sent && writeTurnsRef.current.get(handle) === turn } const groups = resolveNativeChatTranscriptAgent(agentRef.current) === 'codex' @@ -273,8 +276,8 @@ export function useMobileNativeChatAnswerSend(args: { deadline += MOBILE_NATIVE_CHAT_QUESTION_STEP_MS } } - // Superseded on the last key: same as above, the successor owns the surface. - return groups.length > 0 && generationRef.current === generation + // Taken over on the last key: same as above, the successor owns the surface. + return groups.length > 0 && writeTurnsRef.current.get(handle) === turn } finally { // Any accepted key changed the live selector, so a queued replacement // cannot safely apply its from-scratch key plan to that new position.