mirror of
https://github.com/stablyai/orca.git
synced 2026-10-20 08:03:28 +00:00
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.
This commit is contained in:
@@ -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<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> = []
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user