fix(mobile): serialize native chat PTY writes and fence superseded ask keystrokes (STA-3333) (#12502)

* fix(mobile): serialize native chat PTY writes

Two composed native-chat write sequences into one PTY interleaved their
bytes: the per-terminal send-in-flight guard lived inside the image
attachments hook, so ask answers, permission choices, and question
answers wrote straight past it. Move the guard to a shared module-scope
write lock (the terminal outlives any one screen) and take it on all
four paths.

Ask answers additionally queue behind the prior chain's RPC rather than
racing it, and a superseding answer is fenced once a key has actually
landed: an accepted or ambiguously-delivered keystroke already moved the
remote selector, so a replacement's from-scratch key plan would answer
the wrong question. A superseding answer inherits the cancelled chain's
hold (refcounted, last chain out releases) so changing your mind
mid-answer still works, and Stop/cancel stay unguarded so an interrupt
can never deadlock against the send it cancels.

Refs STA-3333.

* fix(mobile): report a fenced native-chat answer instead of dropping it

The fence added for superseding Ask answers returned false with no
onSendError, and the card re-enables on a false result — so a queued
answer vanished with no banner and no toast, looking exactly like a dead
button. Every other bail in answerAsk reports. Keep the generation-
mismatch branch silent: a newer answer owns the error surface.

Also covers the hold-count release, which had no test at all: replacing
it with an unconditional release left all 58 tests green while silently
reopening the terminal mid-sequence — the exact interleave this PR fixes.

* fix(mobile): stop a superseded answer from clearing the fence banner

A chain that finished its key plan reported `true` even after a newer answer
superseded it. The route sends through useNativeChatAcceptedAction, whose
accepted callback retires the send-error banner — and that callback runs after
the successor's fence report, because finishTurn() fires in `finally`, before
the chain's own promise settles. So the successful predecessor deterministically
wiped the fence message the successor had just raised: every healthy write took
that branch, which made the previous commit's report vacuous exactly where it
mattered.

A superseded chain now reports no success, matching every other supersession
checkpoint in this hook.

* fix(mobile): fence on the turn slot, not the generation counter

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.

* fix(mobile): report a fenced answer when a dropped lease bumps the generation

The fence-report guard tested generationRef, which answers "was I
cancelled", not "did a successor replace me". Stop and ask-cancel both
write an Escape that clears the ask card, so their silence is harmless.
A dropped input lease bumps the same counter and writes nothing: the
card stays up with Submit re-enabled while the predecessor's option key
has already advanced the live selector, so the retry double-steps it.

Gate on the turn slot, matching the two success returns.

* test(mobile): cover the turn-slot release after a landed answer

The slot delete in the finally was the one line in this hook no test
killed: without it a landed answer's resolved-false turn stays parked,
so every later answer on that handle reads a fenced predecessor and the
ask card dies after its first use.

* test(mobile): keep PTY write locks terminal-scoped
This commit is contained in:
Brennan Benson 2026-08-04 13:01:39 -07:00 committed by GitHub
parent 9759bd2e76
commit d27d69fbff
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 885 additions and 124 deletions

View File

@ -13,6 +13,11 @@ import {
markMobileNativeChatInputStale,
resetMobileNativeChatStaleInputForTests
} from './mobile-native-chat-stale-input'
import {
acquireMobileNativeChatTerminalWrite,
releaseMobileNativeChatTerminalWrite,
resetMobileNativeChatTerminalWritesForTests
} from './mobile-native-chat-terminal-write-lock'
describe('sendMobileNativeChatPermissionResponse', () => {
it('writes an approval as raw bytes without appending Return', async () => {
@ -64,6 +69,7 @@ describe('useMobileNativeChatPermissionSend', () => {
beforeEach(() => {
globalThis.IS_REACT_ACT_ENVIRONMENT = true
resetMobileNativeChatStaleInputForTests()
resetMobileNativeChatTerminalWritesForTests()
})
afterEach(() => {
@ -102,4 +108,42 @@ describe('useMobileNativeChatPermissionSend', () => {
expect(sendRequest.mock.calls[0]?.[1]).toMatchObject({ text: '1', enter: false })
expect(isMobileNativeChatInputStale('terminal')).toBe(true)
})
it('rejects a choice while another composed write holds the terminal, then recovers', async () => {
const onSendError = vi.fn()
const sendRequest = vi.fn().mockResolvedValue({
ok: true,
result: { send: { handle: 'terminal', accepted: true, bytesWritten: 1 } }
})
function Harness(): null {
respond = useMobileNativeChatPermissionSend({
client: { sendRequest } as unknown as RpcClient,
enabled: true,
handleRef: { current: 'terminal' },
deviceTokenRef: { current: null },
onSendError
})
return null
}
act(() => {
renderer = create(createElement(Harness))
})
// An image paste sequence is mid-flight into the same PTY: the choice
// keystroke must not interleave into it.
expect(acquireMobileNativeChatTerminalWrite('terminal')).toBe(true)
await act(async () => {
await expect(respond?.('1')).resolves.toBe(false)
})
expect(sendRequest).not.toHaveBeenCalled()
expect(onSendError).toHaveBeenCalledWith('Response not sent')
releaseMobileNativeChatTerminalWrite('terminal')
await act(async () => {
await expect(respond?.('1')).resolves.toBe(true)
})
// The choice released its own hold on the way out.
expect(acquireMobileNativeChatTerminalWrite('terminal')).toBe(true)
releaseMobileNativeChatTerminalWrite('terminal')
})
})

View File

@ -4,6 +4,10 @@ import {
sendMobileNativeChatMessageWithOutcome,
type MobileNativeChatSendOutcome
} from './mobile-native-chat-send'
import {
acquireMobileNativeChatTerminalWrite,
releaseMobileNativeChatTerminalWrite
} from './mobile-native-chat-terminal-write-lock'
export function sendMobileNativeChatPermissionResponse(args: {
client: RpcClient
@ -36,15 +40,26 @@ export function useMobileNativeChatPermissionSend(args: {
args.onSendError('Response not sent (disconnected)')
return false
}
// A choice keystroke must not interleave into a mid-flight composed write
// (image paste, paced answer) on the same PTY.
if (!acquireMobileNativeChatTerminalWrite(terminal)) {
args.onSendError('Response not sent')
return false
}
// No stale-input heal here (unlike the text/ask sends): a choice is an
// `enter: false` key for an active overlay that swallows the clear, so it
// would consume the marker still protecting the next real message.
const outcome = await sendMobileNativeChatPermissionResponse({
client: args.client,
terminal,
deviceToken: args.deviceTokenRef.current,
text
})
let outcome: MobileNativeChatSendOutcome
try {
outcome = await sendMobileNativeChatPermissionResponse({
client: args.client,
terminal,
deviceToken: args.deviceTokenRef.current,
text
})
} finally {
releaseMobileNativeChatTerminalWrite(terminal)
}
if (outcome === 'unknown') {
// Why: the response may have been delivered (ack lost / path cutover) —
// a definite "not sent" would invite a double answer.

View File

@ -0,0 +1,20 @@
import { afterEach, describe, expect, it } from 'vitest'
import {
acquireMobileNativeChatTerminalWrite,
releaseMobileNativeChatTerminalWrite,
resetMobileNativeChatTerminalWritesForTests
} from './mobile-native-chat-terminal-write-lock'
describe('mobile-native-chat-terminal-write-lock', () => {
afterEach(resetMobileNativeChatTerminalWritesForTests)
it('allows composed writes on different terminals to proceed concurrently', () => {
expect(acquireMobileNativeChatTerminalWrite('terminal-a')).toBe(true)
expect(acquireMobileNativeChatTerminalWrite('terminal-b')).toBe(true)
expect(acquireMobileNativeChatTerminalWrite('terminal-a')).toBe(false)
releaseMobileNativeChatTerminalWrite('terminal-a')
releaseMobileNativeChatTerminalWrite('terminal-b')
})
})

View File

@ -0,0 +1,25 @@
// Serializes composed native-chat write sequences (clear/paste/settle/submit,
// paced answer keystrokes) per HOST terminal. Two concurrent sequences into one
// PTY interleave their bytes; a second sender must be rejected up front, not
// woven in. Module scope for the same reason as the stale-input marker: the
// terminal outlives any one screen, and independent hooks share the same PTY.
const writeInFlightTerminals = new Set<string>()
/** Claim the terminal for one composed write sequence. False = another
* sequence is mid-flight; the caller must reject its send. */
export function acquireMobileNativeChatTerminalWrite(terminal: string): boolean {
if (writeInFlightTerminals.has(terminal)) {
return false
}
writeInFlightTerminals.add(terminal)
return true
}
export function releaseMobileNativeChatTerminalWrite(terminal: string): void {
writeInFlightTerminals.delete(terminal)
}
/** Test-only: module scope outlives a single test's hooks. */
export function resetMobileNativeChatTerminalWritesForTests(): void {
writeInFlightTerminals.clear()
}

View File

@ -11,7 +11,13 @@ import {
markMobileNativeChatInputStale,
resetMobileNativeChatStaleInputForTests
} from './mobile-native-chat-stale-input'
import {
acquireMobileNativeChatTerminalWrite,
releaseMobileNativeChatTerminalWrite,
resetMobileNativeChatTerminalWritesForTests
} from './mobile-native-chat-terminal-write-lock'
import { useMobileNativeChatAnswerSend } from './use-mobile-native-chat-answer-send'
import { useNativeChatAcceptedAction } from './use-native-chat-action-outcomes'
type AnswerSend = ReturnType<typeof useMobileNativeChatAnswerSend>
@ -40,17 +46,24 @@ describe('useMobileNativeChatAnswerSend', () => {
let mountedClient: RpcClient | null = null
let mountedOnSendError: ((message: string) => void) | null = null
let mountedAgent: AgentType = 'claude'
// The route sends through useNativeChatAcceptedAction, whose accepted callback
// retires the shared send-error banner (use-mobile-native-chat-controller.ts).
let acceptedAnswerAsk: AnswerSend['answerAsk'] | null = null
let onAccepted = vi.fn()
beforeEach(() => {
onAccepted = vi.fn()
vi.useFakeTimers()
globalThis.IS_REACT_ACT_ENVIRONMENT = true
resetMobileNativeChatStaleInputForTests()
resetMobileNativeChatTerminalWritesForTests()
})
afterEach(() => {
act(() => renderer?.unmount())
renderer = null
answerSend = null
acceptedAnswerAsk = null
mountedClient = null
mountedOnSendError = null
mountedAgent = 'claude'
@ -68,6 +81,7 @@ describe('useMobileNativeChatAnswerSend', () => {
streamIdentity: 'host\0worktree\0tab\0session',
onSendError: mountedOnSendError!
})
acceptedAnswerAsk = useNativeChatAcceptedAction(answerSend.answerAsk, onAccepted)
return null
}
@ -396,4 +410,517 @@ describe('useMobileNativeChatAnswerSend', () => {
await expect(result).resolves.toBe(false)
expect(sendRequest).toHaveBeenCalledTimes(1)
})
it('rejects an answer while another composed write holds the terminal', async () => {
const onSendError = vi.fn()
const sendRequest = vi.fn().mockResolvedValue(acceptedResponse())
await mount({ sendRequest } as unknown as RpcClient, onSendError)
// An image paste sequence is mid-flight into the same PTY.
expect(acquireMobileNativeChatTerminalWrite('terminal')).toBe(true)
await expect(answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [1] }])).resolves.toBe(false)
expect(sendRequest).not.toHaveBeenCalled()
expect(onSendError).toHaveBeenCalledWith('Answer not sent')
// Once that sequence releases, answers flow again.
releaseMobileNativeChatTerminalWrite('terminal')
await expect(answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [1] }])).resolves.toBe(true)
})
it('answers again on the same handle after an earlier answer already landed', async () => {
const onSendError = vi.fn()
const sendRequest = vi.fn().mockResolvedValue(acceptedResponse())
await mount({ sendRequest } as unknown as RpcClient, onSendError)
await expect(answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [0] }])).resolves.toBe(true)
// A landed answer resolves its turn FALSE — correct for a queued successor,
// fatal if the turn outlives the chain. Leaving it parked in the slot fences
// every later answer on this handle for the life of the hook, not just an
// overlapping one, so the ask card dies after its first use.
await expect(answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [1] }])).resolves.toBe(true)
expect(sendRequest).toHaveBeenCalledTimes(2)
expect(onSendError).not.toHaveBeenCalled()
})
it('fences a superseding answer after the cancelled chain moved the selector', async () => {
const sendRequest = vi.fn().mockResolvedValue(acceptedResponse())
await mount({ sendRequest } as unknown as RpcClient, vi.fn())
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
let second: Promise<boolean> | undefined
await act(async () => {
first = answerSend?.answerAsk(prompt, [{ indices: [0] }, { indices: [0] }])
})
// The first digit already advanced Claude to q2. Replaying a from-q1 key
// plan now would answer the wrong question.
await act(async () => {
second = answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [1] }])
})
await act(async () => vi.runAllTimersAsync())
await expect(first).resolves.toBe(false)
await expect(second).resolves.toBe(false)
expect(sendRequest).toHaveBeenCalledTimes(1)
// Both chains unwound: the terminal is free for the next composed write.
expect(acquireMobileNativeChatTerminalWrite('terminal')).toBe(true)
releaseMobileNativeChatTerminalWrite('terminal')
})
it('does not write a successor after the prior in-flight key is accepted', async () => {
let resolveFirst: (response: unknown) => void = () => undefined
const sendRequest = vi
.fn()
.mockImplementationOnce(
() =>
new Promise((resolve) => {
resolveFirst = resolve
})
)
.mockResolvedValue(acceptedResponse())
await mount({ sendRequest } as unknown as RpcClient, vi.fn())
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
let second: Promise<boolean> | undefined
await act(async () => {
first = answerSend?.answerAsk(prompt, [{ indices: [0] }, { indices: [0] }])
await Promise.resolve()
})
await act(async () => {
second = answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [1] }])
await Promise.resolve()
})
// The successor is queued behind the in-flight key, not racing it.
expect(sendRequest).toHaveBeenCalledTimes(1)
await act(async () => {
resolveFirst(acceptedResponse())
await Promise.resolve()
})
await expect(first).resolves.toBe(false)
await expect(second).resolves.toBe(false)
expect(sendRequest).toHaveBeenCalledTimes(1)
})
it('lets a queued successor continue after the prior key is definitely rejected', async () => {
let resolveFirst: (response: unknown) => void = () => undefined
const sendRequest = vi
.fn()
.mockImplementationOnce(
() =>
new Promise((resolve) => {
resolveFirst = resolve
})
)
.mockResolvedValue(acceptedResponse())
await mount({ sendRequest } as unknown as RpcClient, vi.fn())
let first: Promise<boolean> | undefined
let second: Promise<boolean> | undefined
await act(async () => {
first = answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [0] }])
await Promise.resolve()
})
await act(async () => {
second = answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [1] }])
await Promise.resolve()
})
await act(async () => {
resolveFirst({
...acceptedResponse(),
result: { send: { accepted: false } }
})
await Promise.resolve()
})
// Nothing landed, so the selector never moved — the successor is safe.
await expect(first).resolves.toBe(false)
await expect(second).resolves.toBe(true)
expect(sendRequest).toHaveBeenCalledTimes(2)
})
it('does not write a successor after the prior delivery becomes ambiguous', async () => {
let rejectFirst: (error: Error) => void = () => undefined
const sendRequest = vi
.fn()
.mockImplementationOnce(
() =>
new Promise((_resolve, reject) => {
rejectFirst = reject
})
)
.mockResolvedValue(acceptedResponse())
await mount({ sendRequest } as unknown as RpcClient, vi.fn())
let first: Promise<boolean> | undefined
let second: Promise<boolean> | undefined
await act(async () => {
first = answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [0] }])
await Promise.resolve()
})
await act(async () => {
second = answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [1] }])
await Promise.resolve()
})
await act(async () => {
rejectFirst(markRpcDeliveryUnknown(new Error('Connection closed')))
await Promise.resolve()
})
// The key may have landed; a blind successor could double-step the selector.
await expect(first).resolves.toBe(false)
await expect(second).resolves.toBe(false)
expect(sendRequest).toHaveBeenCalledTimes(1)
})
it('tells the user when a queued answer is fenced instead of dropping it silently', async () => {
const onSendError = vi.fn()
let rejectFirst: (error: Error) => void = () => undefined
const sendRequest = vi
.fn()
.mockImplementationOnce(
() =>
new Promise((_resolve, reject) => {
rejectFirst = reject
})
)
.mockResolvedValue(acceptedResponse())
await mount({ sendRequest } as unknown as RpcClient, onSendError)
let first: Promise<boolean> | undefined
let second: Promise<boolean> | undefined
await act(async () => {
first = answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [0] }])
await Promise.resolve()
})
await act(async () => {
second = answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [1] }])
await Promise.resolve()
})
await act(async () => {
rejectFirst(markRpcDeliveryUnknown(new Error('Connection closed')))
await Promise.resolve()
})
await expect(first).resolves.toBe(false)
await expect(second).resolves.toBe(false)
// The card re-enables on a false result, so an unreported fence looks exactly
// like a dead button. The superseded chain stays quiet — the newest answer
// owns the error surface, and it is the one the user is waiting on.
expect(onSendError).toHaveBeenCalledTimes(1)
expect(onSendError).toHaveBeenCalledWith('Answer not sent — check chat before retrying')
})
it('tells the user when a dropped lease fences a queued answer whose predecessor landed', 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<boolean> | undefined
let second: Promise<boolean> | undefined
await act(async () => {
first = answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [1] }])
await Promise.resolve()
})
await act(async () => {
second = answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [0] }])
await Promise.resolve()
})
// A transport blip, not a user cancel: unlike Stop and ask-cancel, the lost
// input lease writes no Escape, so the card stays up with Submit re-enabled.
await setEnabled(false)
await act(async () => {
settle[0]!(acceptedResponse())
await Promise.resolve()
})
expect(sendRequest).toHaveBeenCalledTimes(1)
await expect(first).resolves.toBe(false)
await expect(second).resolves.toBe(false)
// The first option key LANDED, so the live selector already moved. Reporting
// nothing invites a retry that double-steps it.
expect(onSendError).toHaveBeenCalledTimes(1)
expect(onSendError).toHaveBeenCalledWith('Answer not sent — check chat before retrying')
})
it('keeps the terminal locked while a queued successor writes', async () => {
const settle: Array<(response: unknown) => void> = []
const sendRequest = vi.fn().mockImplementation(
() =>
new Promise((resolve) => {
settle.push(resolve)
})
)
await mount({ sendRequest } as unknown as RpcClient, vi.fn())
let first: Promise<boolean> | undefined
let second: Promise<boolean> | undefined
await act(async () => {
first = answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [0] }])
await Promise.resolve()
})
await act(async () => {
second = answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [1] }])
await Promise.resolve()
})
expect(sendRequest).toHaveBeenCalledTimes(1)
// Nothing landed, so the successor is cleared to write.
await act(async () => {
settle[0]!({ ...acceptedResponse(), result: { send: { accepted: false } } })
await Promise.resolve()
})
expect(sendRequest).toHaveBeenCalledTimes(2)
// The successor's key is on the wire. The superseded chain unwinding behind it
// must NOT free the terminal, or an image paste interleaves into this sequence.
expect(acquireMobileNativeChatTerminalWrite('terminal')).toBe(false)
await act(async () => {
settle[1]!(acceptedResponse())
await Promise.resolve()
})
await expect(first).resolves.toBe(false)
await expect(second).resolves.toBe(true)
expect(acquireMobileNativeChatTerminalWrite('terminal')).toBe(true)
releaseMobileNativeChatTerminalWrite('terminal')
})
it('does not retire the fence banner when the superseded answer lands', 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<boolean> | undefined
let second: Promise<boolean> | undefined
await act(async () => {
first = acceptedAnswerAsk?.(TABS_OR_SPACES, [{ indices: [0] }])
await Promise.resolve()
})
await act(async () => {
second = acceptedAnswerAsk?.(TABS_OR_SPACES, [{ indices: [1] }])
await Promise.resolve()
})
// The healthy path: the first answer LANDS, which is also the case that fences
// hardest — its key moved the live selector.
await act(async () => {
settle[0]!(acceptedResponse())
await Promise.resolve()
})
await expect(first).resolves.toBe(false)
await expect(second).resolves.toBe(false)
expect(onSendError).toHaveBeenCalledWith('Answer not sent — check chat before retrying')
// A superseded chain reporting success would clear the banner it just raised —
// the accepted hook runs after the fence, so the user would see nothing at all.
expect(onAccepted).not.toHaveBeenCalled()
expect(sendRequest).toHaveBeenCalledTimes(1)
})
it('does not retire the fence banner when a superseded pasted answer lands', 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<boolean> | undefined
let second: Promise<boolean> | undefined
await act(async () => {
first = acceptedAnswerAsk?.(TABS_OR_SPACES, [{ indices: [0] }])
await Promise.resolve()
})
await act(async () => {
second = acceptedAnswerAsk?.(TABS_OR_SPACES, [{ indices: [1] }])
await Promise.resolve()
})
await act(async () => {
settle[0]!(acceptedResponse())
await Promise.resolve()
})
// The pasted shape commits with Enter, so a superseded chain is doubly unsafe
// to report as accepted — the answer it committed is not the one on screen.
await expect(first).resolves.toBe(false)
await expect(second).resolves.toBe(false)
expect(onAccepted).not.toHaveBeenCalled()
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<boolean> | 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<boolean> | 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> = []
const sendRequest = vi.fn().mockImplementation(
() =>
new Promise((resolve) => {
settle.push(resolve)
})
)
await mount({ sendRequest } as unknown as RpcClient, onSendError)
let first: Promise<boolean> | undefined
let second: Promise<boolean> | undefined
let third: Promise<boolean> | undefined
await act(async () => {
first = answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [0] }])
await Promise.resolve()
})
await act(async () => {
second = answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [1] }])
await Promise.resolve()
})
await act(async () => {
third = answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [0] }])
await Promise.resolve()
})
expect(sendRequest).toHaveBeenCalledTimes(1)
await act(async () => {
settle[0]!(acceptedResponse())
await Promise.resolve()
})
await expect(first).resolves.toBe(false)
await expect(second).resolves.toBe(false)
await expect(third).resolves.toBe(false)
// The middle chain sent nothing, so only the verdict it INHERITED can stop the
// third from replaying a from-scratch plan onto the advanced selector.
expect(sendRequest).toHaveBeenCalledTimes(1)
// Only the newest chain owns the error surface.
expect(onSendError).toHaveBeenCalledTimes(1)
expect(onSendError).toHaveBeenCalledWith('Answer not sent — check chat before retrying')
})
it('queues a late third answer behind the successor already on the wire', async () => {
const settle: Array<(response: unknown) => void> = []
const sendRequest = vi.fn().mockImplementation(
() =>
new Promise((resolve) => {
settle.push(resolve)
})
)
await mount({ sendRequest } as unknown as RpcClient, vi.fn())
let first: Promise<boolean> | undefined
let second: Promise<boolean> | undefined
let third: Promise<boolean> | undefined
await act(async () => {
first = answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [0] }])
await Promise.resolve()
})
await act(async () => {
second = answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [1] }])
await Promise.resolve()
})
// Nothing landed, so the successor is cleared and puts its own key on the wire.
await act(async () => {
settle[0]!({ ...acceptedResponse(), result: { send: { accepted: false } } })
await Promise.resolve()
})
expect(sendRequest).toHaveBeenCalledTimes(2)
await act(async () => {
third = answerSend?.answerAsk(TABS_OR_SPACES, [{ indices: [0] }])
await Promise.resolve()
await Promise.resolve()
})
// The first chain unwound while the second was mid-write: it must not have
// dropped the second's turn, or this one writes into the same PTY concurrently.
expect(sendRequest).toHaveBeenCalledTimes(2)
await act(async () => {
settle[1]!(acceptedResponse())
await Promise.resolve()
})
await expect(first).resolves.toBe(false)
await expect(second).resolves.toBe(false)
await expect(third).resolves.toBe(false)
expect(sendRequest).toHaveBeenCalledTimes(2)
})
})

View File

@ -14,6 +14,10 @@ import {
sendMobileNativeChatMessageWithOutcome
} from './mobile-native-chat-send'
import { healMobileNativeChatStaleInput } from './mobile-native-chat-stale-input'
import {
acquireMobileNativeChatTerminalWrite,
releaseMobileNativeChatTerminalWrite
} from './mobile-native-chat-terminal-write-lock'
import {
resolveNativeChatTranscriptAgent,
shouldStepNativeChatAskAnswer
@ -72,6 +76,12 @@ export function useMobileNativeChatAnswerSend(args: {
const generationRef = useRef(0)
const activeRouteRef = useRef({ client, enabled, sessionId, streamIdentity })
activeRouteRef.current = { client, enabled, sessionId, streamIdentity }
// Per-terminal count of this hook's chains sharing one write-lock hold: a
// superseding answer inherits the cancelled chain's hold (it re-enters before
// the old chain unwinds), and only the last chain out releases the lock.
const writeHoldsRef = useRef(new Map<string, number>())
// Successors wait for the prior RPC and inherit any delivery ambiguity.
const writeTurnsRef = useRef(new Map<string, Promise<boolean>>())
const delaysRef = useRef<
Set<{ timer: ReturnType<typeof setTimeout>; resolve: (completed: boolean) => void }>
>(new Set())
@ -103,125 +113,191 @@ export function useMobileNativeChatAnswerSend(args: {
if (!hasAskAnswer(prompt, selections)) {
return false
}
// One composed write sequence per terminal: an answer landing mid-flight
// in an image paste (or vice versa) would interleave bytes into the PTY.
// A superseding answer shares the cancelled chain's hold on this terminal
// (that chain has not unwound to its release yet).
const holds = writeHoldsRef.current
const heldCount = holds.get(handle) ?? 0
if (heldCount === 0 && !acquireMobileNativeChatTerminalWrite(handle)) {
onSendError('Answer not sent')
return false
}
holds.set(handle, heldCount + 1)
const previousTurn = writeTurnsRef.current.get(handle) ?? Promise.resolve(true)
let finishTurn: (safeToContinue: boolean) => void = () => undefined
const turn = new Promise<boolean>((resolve) => {
finishTurn = resolve
})
writeTurnsRef.current.set(handle, turn)
// A new answer supersedes any still-pending keystroke writes.
cancelPending()
const generation = generationRef.current
let sawUnknownOutcome = false
let sawAcceptedGroup = false
// One budget for the whole answer instead of a fresh timeout per keystroke
// group, which let an N-group selector hold the card for N × the send timeout.
// It bounds transport time only: each deliberate pacing wait is credited back
// below, so a long multi-question answer still gets a full budget to write in.
let deadline = openMobileNativeChatSendBudget()
const sendTerminal = async (body: string, enter: boolean): Promise<boolean> => {
const activeRoute = activeRouteRef.current
if (
!activeRoute.enabled ||
activeRoute.client !== client ||
activeRoute.sessionId !== sessionId ||
activeRoute.streamIdentity !== streamIdentity ||
handleRef.current !== handle
) {
return false
}
const outcome = await sendMobileNativeChatMessageWithOutcome({
client,
terminal: handle,
text: body,
enter,
deadline,
...(deviceTokenRef.current
? { mobileClient: { id: deviceTokenRef.current, type: 'mobile' } }
: {})
})
if (outcome === 'unknown') {
sawUnknownOutcome = true
}
if (outcome === 'accepted') {
sawAcceptedGroup = true
}
return outcome === 'accepted'
}
const wait = (ms: number): Promise<boolean> =>
new Promise((resolve) => {
const delay = {
timer: setTimeout(() => {
delaysRef.current.delete(delay)
resolve(generationRef.current === generation)
}, ms),
resolve
}
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.
onSendError(
sawAcceptedGroup
? 'Answer partly sent — check chat before retrying'
: sawUnknownOutcome
? 'Answer unconfirmed — check chat before retrying'
: 'Answer not sent'
)
}
return false
}
// Grok commits pasted labels; Claude and Codex need their selector-specific
// keystrokes paced so each step renders before the next lands.
if (!shouldStepNativeChatAskAnswer(agentRef.current)) {
// This shape pastes the label into the composer and commits it, so an
// orphaned image paste would be submitted along with the answer (#10228).
// The selector shapes below deliberately skip the heal: their keys are
// `enter: false` for an active overlay, and a single-select answer is a
// bare option digit that cannot submit the line at all, so clearing there
// would consume the marker still protecting the next real message.
// Desktop splits it identically — use-native-chat-interactive-send.ts
// routes only the pasted-label shape through the clearing sender.
if (
!(await healMobileNativeChatStaleInput({
client,
terminal: handle,
deviceToken: deviceTokenRef.current,
deadline
}))
) {
if (generationRef.current === generation) {
onSendError('Answer not sent')
let predecessorSafe = true
try {
predecessorSafe = await previousTurn
if (!predecessorSafe) {
// Fenced. Report it: the card re-enables on a false result, so silence
// here is indistinguishable from a dead button. "Check chat" rather than
// a bare "not sent" because the PREVIOUS answer's keys may have landed.
// Gate on the turn slot, not the generation: a dropped input lease bumps
// the generation without writing the Escape that Stop and ask-cancel do,
// so the card is still up and silence there strands an advanced selector.
if (writeTurnsRef.current.get(handle) === turn) {
onSendError('Answer not sent — check chat before retrying')
}
return false
}
// Superseded by a newer answer, which owns the error surface from here.
if (generationRef.current !== generation) {
return false
}
return (await sendTerminal(formatAskAnswer(prompt, selections), true)) || fail()
}
const groups =
resolveNativeChatTranscriptAgent(agentRef.current) === 'codex'
? buildCodexAskAnswerKeys(prompt, selections)
: buildAskAnswerKeys(prompt, selections)
for (let index = 0; index < groups.length; index += 1) {
if (generationRef.current !== generation) {
return false
}
const group = groups[index]!
const body = 'raw' in group ? group.raw : sanitizeAskFreeText(group.text)
if (!(await sendTerminal(body, false))) {
return fail()
}
if (index < groups.length - 1) {
if (!(await wait(MOBILE_NATIVE_CHAT_QUESTION_STEP_MS))) {
// One budget for the whole answer instead of a fresh timeout per keystroke
// group, which let an N-group selector hold the card for N × the send timeout.
// It bounds transport time only: each deliberate pacing wait is credited back
// below, so a long multi-question answer still gets a full budget to write in.
let deadline = openMobileNativeChatSendBudget()
const sendTerminal = async (body: string, enter: boolean): Promise<boolean> => {
const activeRoute = activeRouteRef.current
if (
!activeRoute.enabled ||
activeRoute.client !== client ||
activeRoute.sessionId !== sessionId ||
activeRoute.streamIdentity !== streamIdentity ||
handleRef.current !== handle
) {
return false
}
// Pacing is deliberate, not transport latency — don't charge it to the budget.
deadline += MOBILE_NATIVE_CHAT_QUESTION_STEP_MS
const outcome = await sendMobileNativeChatMessageWithOutcome({
client,
terminal: handle,
text: body,
enter,
deadline,
...(deviceTokenRef.current
? { mobileClient: { id: deviceTokenRef.current, type: 'mobile' } }
: {})
})
if (outcome === 'unknown') {
sawUnknownOutcome = true
}
if (outcome === 'accepted') {
sawAcceptedGroup = true
}
return outcome === 'accepted'
}
const wait = (ms: number): Promise<boolean> => {
// Already superseded: don't hold the successor for a full pacing step
// waiting on a timer whose only job is to report the cancellation.
if (generationRef.current !== generation) {
return Promise.resolve(false)
}
return new Promise((resolve) => {
const delay = {
timer: setTimeout(() => {
delaysRef.current.delete(delay)
resolve(generationRef.current === generation)
}, ms),
resolve
}
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.
onSendError(
sawAcceptedGroup
? 'Answer partly sent — check chat before retrying'
: sawUnknownOutcome
? 'Answer unconfirmed — check chat before retrying'
: 'Answer not sent'
)
}
return false
}
// Grok commits pasted labels; Claude and Codex need their selector-specific
// keystrokes paced so each step renders before the next lands.
if (!shouldStepNativeChatAskAnswer(agentRef.current)) {
// This shape pastes the label into the composer and commits it, so an
// orphaned image paste would be submitted along with the answer (#10228).
// The selector shapes below deliberately skip the heal: their keys are
// `enter: false` for an active overlay, and a single-select answer is a
// bare option digit that cannot submit the line at all, so clearing there
// would consume the marker still protecting the next real message.
// Desktop splits it identically — use-native-chat-interactive-send.ts
// routes only the pasted-label shape through the clearing sender.
if (
!(await healMobileNativeChatStaleInput({
client,
terminal: handle,
deviceToken: deviceTokenRef.current,
deadline
}))
) {
if (generationRef.current === generation) {
onSendError('Answer not sent')
}
return false
}
if (generationRef.current !== generation) {
return false
}
// 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 && writeTurnsRef.current.get(handle) === turn
}
const groups =
resolveNativeChatTranscriptAgent(agentRef.current) === 'codex'
? buildCodexAskAnswerKeys(prompt, selections)
: buildAskAnswerKeys(prompt, selections)
for (let index = 0; index < groups.length; index += 1) {
if (generationRef.current !== generation) {
return false
}
const group = groups[index]!
const body = 'raw' in group ? group.raw : sanitizeAskFreeText(group.text)
if (!(await sendTerminal(body, false))) {
return fail()
}
if (index < groups.length - 1) {
if (!(await wait(MOBILE_NATIVE_CHAT_QUESTION_STEP_MS))) {
return false
}
// Pacing is deliberate, not transport latency — don't charge it to the budget.
deadline += MOBILE_NATIVE_CHAT_QUESTION_STEP_MS
}
}
// 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.
finishTurn(predecessorSafe && !sawUnknownOutcome && !sawAcceptedGroup)
if (writeTurnsRef.current.get(handle) === turn) {
writeTurnsRef.current.delete(handle)
}
// Last chain out releases; a superseded chain unwinding late must not
// free the lock out from under the successor sharing its hold.
const remaining = (holds.get(handle) ?? 1) - 1
if (remaining <= 0) {
holds.delete(handle)
releaseMobileNativeChatTerminalWrite(handle)
} else {
holds.set(handle, remaining)
}
}
return groups.length > 0
},
[
agentRef,

View File

@ -5,6 +5,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import type { RpcClient } from '../transport/rpc-client'
import type { RpcResponse, RpcSuccess } from '../transport/types'
import { resetMobileNativeChatStaleInputForTests } from './mobile-native-chat-stale-input'
import { resetMobileNativeChatTerminalWritesForTests } from './mobile-native-chat-terminal-write-lock'
import { useMobileNativeChatImageAttachments } from './use-mobile-native-chat-image-attachments'
// Fully stub the picker so the real expo/react-native chain never loads under
@ -88,9 +89,10 @@ describe('useMobileNativeChatImageAttachments', () => {
beforeEach(() => {
globalThis.IS_REACT_ACT_ENVIRONMENT = true
pick.mockReset()
// Stale markers live at module scope now (they outlive the screen), so they
// also outlive a test.
// Stale markers and write locks live at module scope (they outlive the
// screen), so they also outlive a test.
resetMobileNativeChatStaleInputForTests()
resetMobileNativeChatTerminalWritesForTests()
})
afterEach(() => {
act(() => renderer?.unmount())

View File

@ -26,6 +26,10 @@ import {
isMobileNativeChatInputStale,
markMobileNativeChatInputStale
} from './mobile-native-chat-stale-input'
import {
acquireMobileNativeChatTerminalWrite,
releaseMobileNativeChatTerminalWrite
} from './mobile-native-chat-terminal-write-lock'
type CurrentRef<T> = { readonly current: T }
type ShowToast = (message: string, durationMs?: number) => void
@ -127,8 +131,6 @@ export function useMobileNativeChatImageAttachments({
// checked 'connected' at entry, so only a ref can see a mid-upload disconnect.
const connStateRef = useRef(connState)
connStateRef.current = connState
// Serialize clear/paste/submit ownership per terminal while allowing other tabs to send.
const sendInFlightTerminalsRef = useRef(new Set<string>())
const attachments = (scopeKey ? attachmentsByScope[scopeKey] : undefined) ?? NO_ATTACHMENTS
@ -219,15 +221,15 @@ export function useMobileNativeChatImageAttachments({
const sendNativeChat = useCallback(
async (text: string): Promise<boolean> => {
// Serialize clear/paste/submit ownership per terminal while allowing other
// tabs to send. Shared with the prompt-card writes (answer/permission), so
// a card tap can't interleave into a mid-flight paste sequence either.
const operationTerminal = activeHandleRef.current
if (operationTerminal && sendInFlightTerminalsRef.current.has(operationTerminal)) {
if (operationTerminal && !acquireMobileNativeChatTerminalWrite(operationTerminal)) {
onError?.()
onSendError('Message not sent')
return false
}
if (operationTerminal) {
sendInFlightTerminalsRef.current.add(operationTerminal)
}
// One budget for the whole user action. The paste loop, the settle, and the
// text body that follows are a single send from the composer's point of view;
// opening a budget per leg let `sending` run to twice the stated ceiling.
@ -347,7 +349,7 @@ export function useMobileNativeChatImageAttachments({
}
} finally {
if (operationTerminal) {
sendInFlightTerminalsRef.current.delete(operationTerminal)
releaseMobileNativeChatTerminalWrite(operationTerminal)
}
}
},

View File

@ -19,6 +19,11 @@ vi.mock('./mobile-native-chat-stale-input', () => ({
}))
import { useMobileNativeChatMessageSend } from './use-mobile-native-chat-message-send'
import {
acquireMobileNativeChatTerminalWrite,
releaseMobileNativeChatTerminalWrite,
resetMobileNativeChatTerminalWritesForTests
} from './mobile-native-chat-terminal-write-lock'
import { buildAgentTuiClearInputForText } from '../../../src/shared/agent-tui-input-clear'
type Send = ReturnType<typeof useMobileNativeChatMessageSend>
@ -28,6 +33,7 @@ const DRAFT = 'Linked Linear issue: ABC-123\nhttps://linear.app/x/issue/ABC-123'
describe('useMobileNativeChatMessageSend', () => {
let renderer: ReactTestRenderer | null = null
let api: Send | null = null
let onSendError = vi.fn()
const mount = (
readSeededLaunchDraftSeed: () => { text: string; createdAt: number | null } | null
@ -44,7 +50,7 @@ describe('useMobileNativeChatMessageSend', () => {
restoreRejectedDraft: () => {},
acceptSend: () => {},
holdUnconfirmedSend: () => {},
onSendError: () => {}
onSendError
})
return null
}
@ -70,6 +76,8 @@ describe('useMobileNativeChatMessageSend', () => {
sendWithOutcome.mockResolvedValue('accepted')
clearInputWrite.mockReset()
clearInputWrite.mockResolvedValue(true)
onSendError = vi.fn()
resetMobileNativeChatTerminalWritesForTests()
})
afterEach(() => {
act(() => {
@ -173,4 +181,27 @@ describe('useMobileNativeChatMessageSend', () => {
})
expect(sentArgs().resolvedLaunchDraft).toBeUndefined()
})
it('rejects a question answer while another composed write holds the terminal', async () => {
mount(() => null)
// An image paste sequence is mid-flight into the same PTY.
expect(acquireMobileNativeChatTerminalWrite('term')).toBe(true)
let result: boolean | undefined
await act(async () => {
result = await api!.answerQuestion('1')
})
expect(result).toBe(false)
expect(sendWithOutcome).not.toHaveBeenCalled()
expect(onSendError).toHaveBeenCalledWith('Answer not sent')
releaseMobileNativeChatTerminalWrite('term')
await act(async () => {
result = await api!.answerQuestion('1')
})
expect(result).toBe(true)
// The answer released its own hold on the way out.
expect(acquireMobileNativeChatTerminalWrite('term')).toBe(true)
releaseMobileNativeChatTerminalWrite('term')
})
})

View File

@ -7,6 +7,10 @@ import {
type MobileNativeChatSendOutcome
} from './mobile-native-chat-send'
import { healMobileNativeChatStaleInput } from './mobile-native-chat-stale-input'
import {
acquireMobileNativeChatTerminalWrite,
releaseMobileNativeChatTerminalWrite
} from './mobile-native-chat-terminal-write-lock'
import type { MobileNativeChatSendOrigin } from './use-mobile-native-chat-drafts'
import type { MobileNativeChatLaunchDraftSeed } from './use-mobile-native-chat-launch-draft-seed'
import { buildAgentTuiClearInputForText } from '../../../src/shared/agent-tui-input-clear'
@ -210,11 +214,26 @@ export function useMobileNativeChatMessageSend(args: {
[sendWithOutcome]
)
// A question answer is not composer text, so it never syncs the draft.
// A question answer is not composer text, so it never syncs the draft. It
// reaches this send directly (not through the image hook's locked path), so
// it takes the per-terminal write lock itself: an answer landing mid-flight
// in an image paste sequence would interleave bytes into the PTY.
const answerQuestion = useCallback(
async (text: string): Promise<boolean> =>
(await sendMessage(text, undefined, false)) !== 'rejected',
[sendMessage]
async (text: string): Promise<boolean> => {
const terminal = handleRef.current
if (terminal && !acquireMobileNativeChatTerminalWrite(terminal)) {
onSendError('Answer not sent')
return false
}
try {
return (await sendMessage(text, undefined, false)) !== 'rejected'
} finally {
if (terminal) {
releaseMobileNativeChatTerminalWrite(terminal)
}
}
},
[handleRef, onSendError, sendMessage]
)
return { send, sendWithOutcome, answerQuestion }