From 8be97798e9c76dbb3594df50beeedcab41adc057 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 2 Aug 2026 17:47:59 -0700 Subject: [PATCH] fix(mobile): serialize repeated IME composition flushes --- .../app/h/[hostId]/session/[worktreeId].tsx | 23 +++---- ...mobile-attachment-input-lease-gate.test.ts | 20 +++++++ .../session/use-mobile-terminal-paste.test.ts | 46 ++++++++++++++ .../terminal-live-dictation-routing.test.ts | 60 ++++++++++++++++--- .../terminal-live-dictation-routing.ts | 30 ++++++++++ .../src/terminal/terminal-live-ime-state.ts | 25 ++++++-- .../use-terminal-live-input-commit.test.ts | 45 +++++++++++++- .../use-terminal-live-input-commit.ts | 60 +++++++++++++------ .../use-terminal-live-pending-input-flush.ts | 18 ++++-- 9 files changed, 276 insertions(+), 51 deletions(-) diff --git a/mobile/app/h/[hostId]/session/[worktreeId].tsx b/mobile/app/h/[hostId]/session/[worktreeId].tsx index 454ac794fc9..e7743cd204f 100644 --- a/mobile/app/h/[hostId]/session/[worktreeId].tsx +++ b/mobile/app/h/[hostId]/session/[worktreeId].tsx @@ -139,6 +139,7 @@ import { import { normalizeTerminalTextInput } from '../../../../src/terminal/terminal-text-input-normalization' import { appendBufferedDictation, + insertLiveDictationTranscript, routeDictationTranscript } from '../../../../src/terminal/terminal-live-dictation-routing' import { countTerminalGestureInputSequences } from '../../../../src/terminal/terminal-gesture-input' @@ -1236,20 +1237,14 @@ export default function SessionScreen() { if (!insertHandle) { return } - void (async () => { - const flushedPendingInput = await flushPendingLiveInputBeforeExternalSend(insertHandle) - if (!flushedPendingInput) { - showToast('Dictation insert canceled', 1500) - return - } - const sent = await sendLiveTerminalInput(insertHandle, route.text) - if (sent) { - showToast('Dictation inserted') - } else { - triggerError() - showToast('Dictation insert canceled', 1500) - } - })() + void insertLiveDictationTranscript({ + flushPendingInput: flushPendingLiveInputBeforeExternalSend, + handle: insertHandle, + onSendError: triggerError, + sendInput: sendLiveTerminalInput, + showToast, + text: route.text + }) return } setInput((current) => appendBufferedDictation(current, route.text)) diff --git a/mobile/src/session/use-mobile-attachment-input-lease-gate.test.ts b/mobile/src/session/use-mobile-attachment-input-lease-gate.test.ts index 3ff908f9820..6e7a8c6ad4d 100644 --- a/mobile/src/session/use-mobile-attachment-input-lease-gate.test.ts +++ b/mobile/src/session/use-mobile-attachment-input-lease-gate.test.ts @@ -116,6 +116,26 @@ describe('useMobileAttachmentInputLeaseGate', () => { expect(showToast).toHaveBeenCalledWith('Attach canceled before send', 1500) }) + it('keeps concurrent attachments behind their pending-input waits', async () => { + const refs = baseRefs() + const showToast = vi.fn() + const releaseWaits: Array<(flushed: boolean) => void> = [] + const flushPendingInput = vi.fn( + async () => new Promise((resolve) => releaseWaits.push(resolve)) + ) + const { gate } = renderGate({ ...refs, flushPendingInput, showToast }) + + const attachments = [gate()('terminal-1'), gate()('terminal-1')] + await vi.waitFor(() => expect(flushPendingInput).toHaveBeenCalledTimes(2)) + + for (const release of releaseWaits) { + release(true) + } + + await expect(Promise.all(attachments)).resolves.toEqual([true, true]) + expect(showToast).not.toHaveBeenCalled() + }) + it('surfaces cancellation when the target changes while waiting for the lease', async () => { const refs = baseRefs() refs.leaseReady.current = false diff --git a/mobile/src/session/use-mobile-terminal-paste.test.ts b/mobile/src/session/use-mobile-terminal-paste.test.ts index 04d4137c5f1..b1eb077cd0e 100644 --- a/mobile/src/session/use-mobile-terminal-paste.test.ts +++ b/mobile/src/session/use-mobile-terminal-paste.test.ts @@ -80,4 +80,50 @@ describe('useMobileTerminalPaste', () => { expect(onError).toHaveBeenCalledOnce() expect(showToast).toHaveBeenCalledWith('Paste canceled before send', 1500) }) + + it('keeps concurrent pastes behind their pending-input waits', async () => { + const sendRequest = vi.fn() + const client = { sendRequest } as unknown as RpcClient + const releaseWaits: Array<(flushed: boolean) => void> = [] + const flushPendingLiveInputBeforeExternalSend = vi.fn( + async () => new Promise((resolve) => releaseWaits.push(resolve)) + ) + let paste: () => Promise = async () => undefined + + function Probe(): null { + paste = useMobileTerminalPaste({ + activeHandle: 'terminal-a', + activeHandleRef: { current: 'terminal-a' }, + activeSessionTabTypeRef: { current: 'terminal' }, + canSend: true, + client, + clientRef: { current: client }, + connState: 'connected', + connStateRef: { current: 'connected' }, + deviceTokenRef: { current: null }, + flushPendingLiveInputBeforeExternalSend, + getActiveWorktreeConnectionId: async () => null, + onError: vi.fn(), + onSuccess: vi.fn(), + ptyModesRef: { current: new Map() }, + refreshCanPaste: vi.fn(), + showToast: vi.fn() + }) + return null + } + + act(() => { + renderer = create(createElement(Probe)) + }) + const pastes = [paste(), paste()] + await vi.waitFor(() => expect(flushPendingLiveInputBeforeExternalSend).toHaveBeenCalledTimes(2)) + expect(sendRequest).not.toHaveBeenCalled() + + for (const release of releaseWaits) { + release(true) + } + await Promise.all(pastes) + + expect(sendRequest).toHaveBeenCalledTimes(2) + }) }) diff --git a/mobile/src/terminal/terminal-live-dictation-routing.test.ts b/mobile/src/terminal/terminal-live-dictation-routing.test.ts index a50c4eae5da..76da117e200 100644 --- a/mobile/src/terminal/terminal-live-dictation-routing.test.ts +++ b/mobile/src/terminal/terminal-live-dictation-routing.test.ts @@ -1,15 +1,10 @@ -import { readFileSync } from 'node:fs' -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import { appendBufferedDictation, + insertLiveDictationTranscript, routeDictationTranscript } from './terminal-live-dictation-routing' -const sessionRouteSource = readFileSync( - new URL('../../app/h/[hostId]/session/[worktreeId].tsx', import.meta.url), - 'utf8' -) - describe('terminal live dictation routing', () => { it('routes to a direct live insert when live input is active', () => { expect(routeDictationTranscript('hello world', true)).toEqual({ @@ -35,7 +30,54 @@ describe('terminal live dictation routing', () => { expect(appendBufferedDictation('ls -la ', 'in src')).toBe('ls -la in src') }) - it('surfaces a canceled completed transcript instead of dropping it silently', () => { - expect(sessionRouteSource).toContain("showToast('Dictation insert canceled', 1500)") + it('surfaces a canceled completed transcript instead of dropping it silently', async () => { + const sendInput = vi.fn(async () => true) + const showToast = vi.fn() + + await expect( + insertLiveDictationTranscript({ + flushPendingInput: async () => false, + handle: 'terminal-a', + onSendError: vi.fn(), + sendInput, + showToast, + text: 'dictated text' + }) + ).resolves.toBe(false) + + expect(sendInput).not.toHaveBeenCalled() + expect(showToast).toHaveBeenCalledWith('Dictation insert canceled', 1500) + }) + + it('keeps concurrent transcripts behind their pending-input waits', async () => { + const releaseWaits: Array<(flushed: boolean) => void> = [] + const flushPendingInput = vi.fn( + async () => new Promise((resolve) => releaseWaits.push(resolve)) + ) + const sendInput = vi.fn(async () => true) + const showToast = vi.fn() + const insert = (text: string): Promise => + insertLiveDictationTranscript({ + flushPendingInput, + handle: 'terminal-a', + onSendError: vi.fn(), + sendInput, + showToast, + text + }) + + const inserts = [insert('first'), insert('second')] + await vi.waitFor(() => expect(flushPendingInput).toHaveBeenCalledTimes(2)) + expect(sendInput).not.toHaveBeenCalled() + + for (const release of releaseWaits) { + release(true) + } + + await expect(Promise.all(inserts)).resolves.toEqual([true, true]) + expect(sendInput.mock.calls).toEqual([ + ['terminal-a', 'first'], + ['terminal-a', 'second'] + ]) }) }) diff --git a/mobile/src/terminal/terminal-live-dictation-routing.ts b/mobile/src/terminal/terminal-live-dictation-routing.ts index 91776afdda5..66813df663a 100644 --- a/mobile/src/terminal/terminal-live-dictation-routing.ts +++ b/mobile/src/terminal/terminal-live-dictation-routing.ts @@ -23,3 +23,33 @@ export function appendBufferedDictation(current: string, transcript: string): st } return `${current.trimEnd()} ${transcript}` } + +type InsertLiveDictationTranscriptOptions = { + readonly flushPendingInput: (handle: string) => Promise + readonly handle: string + readonly onSendError: () => void + readonly sendInput: (handle: string, text: string) => Promise + readonly showToast: (message: string, durationMs?: number) => void + readonly text: string +} + +export async function insertLiveDictationTranscript({ + flushPendingInput, + handle, + onSendError, + sendInput, + showToast, + text +}: InsertLiveDictationTranscriptOptions): Promise { + if (!(await flushPendingInput(handle))) { + showToast('Dictation insert canceled', 1500) + return false + } + if (await sendInput(handle, text)) { + showToast('Dictation inserted') + return true + } + onSendError() + showToast('Dictation insert canceled', 1500) + return false +} diff --git a/mobile/src/terminal/terminal-live-ime-state.ts b/mobile/src/terminal/terminal-live-ime-state.ts index dd519ca7faa..4a72d93567a 100644 --- a/mobile/src/terminal/terminal-live-ime-state.ts +++ b/mobile/src/terminal/terminal-live-ime-state.ts @@ -3,19 +3,24 @@ export type TerminalLiveImeBoundary = { readonly handle: string } +type TerminalLiveImeOwner = TerminalLiveImeBoundary & { + readonly epoch: number +} + type TerminalLiveImeWaiter = { - readonly boundary: TerminalLiveImeBoundary + readonly owner: TerminalLiveImeOwner readonly promise: Promise readonly resolve: (committed: boolean) => void } export type TerminalLiveImeState = { - owner: TerminalLiveImeBoundary | null + epoch: number + owner: TerminalLiveImeOwner | null waiter: TerminalLiveImeWaiter | null } export function createTerminalLiveImeState(): TerminalLiveImeState { - return { owner: null, waiter: null } + return { epoch: 0, owner: null, waiter: null } } export function isSameTerminalLiveImeBoundary( @@ -33,7 +38,15 @@ export function beginTerminalLiveImeComposition( return } invalidateTerminalLiveImeComposition(state) - state.owner = boundary + state.epoch += 1 + state.owner = { ...boundary, epoch: state.epoch } +} + +function isSameTerminalLiveImeOwner( + left: TerminalLiveImeOwner, + right: TerminalLiveImeOwner +): boolean { + return left.epoch === right.epoch && isSameTerminalLiveImeBoundary(left, right) } export function waitForTerminalLiveImeComposition( @@ -43,14 +56,14 @@ export function waitForTerminalLiveImeComposition( if (!state.owner || !isSameTerminalLiveImeBoundary(state.owner, boundary)) { return null } - if (state.waiter && isSameTerminalLiveImeBoundary(state.waiter.boundary, boundary)) { + if (state.waiter && isSameTerminalLiveImeOwner(state.waiter.owner, state.owner)) { return state.waiter.promise } let resolveWaiter: (committed: boolean) => void = () => undefined const promise = new Promise((resolve) => { resolveWaiter = resolve }) - state.waiter = { boundary, promise, resolve: resolveWaiter } + state.waiter = { owner: state.owner, promise, resolve: resolveWaiter } return promise } diff --git a/mobile/src/terminal/use-terminal-live-input-commit.test.ts b/mobile/src/terminal/use-terminal-live-input-commit.test.ts index d2c08bcf9a1..697d6bbf45d 100644 --- a/mobile/src/terminal/use-terminal-live-input-commit.test.ts +++ b/mobile/src/terminal/use-terminal-live-input-commit.test.ts @@ -15,6 +15,7 @@ type TerminalLiveInputCommitHarness = { readonly setActiveHandle: (next: string) => void readonly setActiveSessionTabType: (next: string | undefined) => void readonly setConnected: (next: boolean) => void + readonly setSendImplementation: (next: TerminalLiveInputSender | null) => void readonly setSendResult: (next: boolean) => void readonly unmount: () => void } @@ -53,11 +54,14 @@ function createTerminalLiveInputCommitHarness({ const sent: string[] = [] const sentByHandle: { bytes: string; handle: string }[] = [] let currentSendResult = sendResult + let currentSendImplementation: TerminalLiveInputSender | null = null const sendLiveTerminalInputRef: RefObject = { current: async (handle, bytes) => { sent.push(bytes) sentByHandle.push({ bytes, handle }) - return currentSendResult + return currentSendImplementation + ? currentSendImplementation(handle, bytes) + : currentSendResult } } // Refs never re-render; only these variables re-run the hook's clear effects. @@ -129,6 +133,9 @@ function createTerminalLiveInputCommitHarness({ renderer?.update(createElement(Harness)) }) }, + setSendImplementation: (next: TerminalLiveInputSender | null): void => { + currentSendImplementation = next + }, setSendResult: (next: boolean): void => { currentSendResult = next }, @@ -336,6 +343,42 @@ describe('terminal live input commit hook', () => { expect(harness.sent).toEqual(['仮名']) }) + it('Given a newer composition starts during an older flush Then preserves and waits for it', async () => { + const harness = createTerminalLiveInputCommitHarness() + let releaseFirstSend: (sent: boolean) => void = () => undefined + const firstSend = new Promise((resolve) => { + releaseFirstSend = resolve + }) + let sendCount = 0 + harness.setSendImplementation(async () => { + sendCount += 1 + return sendCount === 1 ? firstSend : true + }) + changeLiveInput(harness.handlers, 'かな', true) + const flush = harness.handlers.flushPendingLiveInputBeforeExternalSend('terminal-a') + let flushSettled = false + void flush.then(() => { + flushSettled = true + }) + changeLiveInput(harness.handlers, '仮名', false) + await vi.waitFor(() => expect(harness.sent).toEqual(['仮名'])) + + changeLiveInput(harness.handlers, '仮名か', true) + releaseFirstSend(true) + await act(async () => { + await Promise.resolve() + await Promise.resolve() + }) + expect(harness.captures.at(-1)).toBe('仮名か') + expect(flushSettled).toBe(false) + + changeLiveInput(harness.handlers, '仮名漢字', false) + + await expect(flush).resolves.toBe(true) + expect(harness.sent).toEqual(['仮名', '漢字']) + expect(harness.captures.at(-1)).toBe('') + }) + it('Given a deferred external send When the terminal switches Then cancels the original target', async () => { const harness = createTerminalLiveInputCommitHarness() changeLiveInput(harness.handlers, 'かな', true) diff --git a/mobile/src/terminal/use-terminal-live-input-commit.ts b/mobile/src/terminal/use-terminal-live-input-commit.ts index 45d0169c156..c16d3413886 100644 --- a/mobile/src/terminal/use-terminal-live-input-commit.ts +++ b/mobile/src/terminal/use-terminal-live-input-commit.ts @@ -127,24 +127,50 @@ export function useTerminalLiveInputCommit({ const flushPendingLiveInputBeforeExternalSend = useCallback( async (handle: string): Promise => { const boundary = { generation: liveInputGenerationRef.current, handle } - const compositionWait = waitForTerminalLiveImeComposition( - liveInputImeStateRef.current, - boundary - ) - if (compositionWait && !(await compositionWait)) { - return false + const imeState = liveInputImeStateRef.current + while (boundary.generation === liveInputGenerationRef.current) { + if ( + activeHandleRef.current !== handle || + (activeSessionTabTypeRef.current != null && + activeSessionTabTypeRef.current !== 'terminal') || + !liveInputTerminalHandlesRef.current.has(handle) + ) { + return false + } + const compositionWait = waitForTerminalLiveImeComposition(imeState, boundary) + if (compositionWait && !(await compositionWait)) { + return false + } + if (boundary.generation !== liveInputGenerationRef.current) { + return false + } + const compositionOwner = imeState.owner + if (compositionOwner) { + if (!isSameTerminalLiveImeBoundary(compositionOwner, boundary)) { + return false + } + continue + } + const compositionEpoch = imeState.epoch + const pendingHandle = pendingLiveInputHandleRef.current + if (pendingHandle && pendingHandle !== handle) { + clearPendingLiveInputMirror() + return waitForPendingLiveInputFlush() + } + // Why: external bytes land after every composition that began before + // the flush settled; older cleanup must not erase a newer composition. + const flushed = + pendingHandle === handle + ? await flushPendingLiveInputText(handle, () => imeState.epoch === compositionEpoch) + : await waitForPendingLiveInputFlush() + if (!flushed) { + return false + } + if (imeState.epoch === compositionEpoch) { + return true + } } - const pendingHandle = pendingLiveInputHandleRef.current - if (pendingHandle && pendingHandle !== handle) { - clearPendingLiveInputMirror() - return waitForPendingLiveInputFlush() - } - // Why: external bytes (dictation/paste) land after the field's echo on the - // PTY; the field session must fully end or later diffs would erase them. - if (pendingHandle === handle) { - return flushPendingLiveInputText(handle) - } - return waitForPendingLiveInputFlush() + return false }, [clearPendingLiveInputMirror, flushPendingLiveInputText, waitForPendingLiveInputFlush] ) diff --git a/mobile/src/terminal/use-terminal-live-pending-input-flush.ts b/mobile/src/terminal/use-terminal-live-pending-input-flush.ts index f6239312947..106c657314e 100644 --- a/mobile/src/terminal/use-terminal-live-pending-input-flush.ts +++ b/mobile/src/terminal/use-terminal-live-pending-input-flush.ts @@ -23,7 +23,10 @@ type TerminalLivePendingInputFlushOptions = { type TerminalLivePendingInputFlush = { readonly applyLiveInputMirror: (handle: string, fieldText: string) => void readonly clearPendingLiveInputCommit: () => void - readonly flushPendingLiveInputText: (expectedHandle: string | null) => Promise + readonly flushPendingLiveInputText: ( + expectedHandle: string | null, + canClearPendingState?: () => boolean + ) => Promise readonly heldLiveInputTextRef: RefObject readonly pendingLiveInputHandleRef: RefObject readonly sentLiveInputTextRef: RefObject @@ -131,13 +134,18 @@ export function useTerminalLivePendingInputFlush({ ) const flushPendingLiveInputText = useCallback( - async (expectedHandle: string | null): Promise => { + async ( + expectedHandle: string | null, + canClearPendingState: () => boolean = () => true + ): Promise => { const handle = pendingLiveInputHandleRef.current if (!handle) { return waitForPendingLiveInputFlush() } if (expectedHandle !== null && handle !== expectedHandle) { - clearPendingLiveInputCommit() + if (canClearPendingState()) { + clearPendingLiveInputCommit() + } return waitForPendingLiveInputFlush() } @@ -149,7 +157,9 @@ export function useTerminalLivePendingInputFlush({ // Why: an explicit flush ends the field's editing session; the echoed PTY // text stays, so local mirror state must restart from empty. - clearPendingLiveInputCommit() + if (canClearPendingState()) { + clearPendingLiveInputCommit() + } return result }, [clearPendingLiveInputCommit, runMirrorStep, waitForPendingLiveInputFlush]