fix(mobile): only the owning start rolls its capture back

There is one capture seam and it carries no start identity, so round 1's
rollback let a stale start's rejection end a live newer dictation: A opens
the capture and waits on the desktop, the user cancels, B starts and is
recording, A's request finally rejects and ends B's microphone and hands
back B's screen. The rollback now runs only while this start is still the
current one, which is what `wasCurrent` on the line above already reads; a
stale failure still cancels its own desktop session and touches nothing
else. Past the generation the capture was either already ended by whatever
superseded this start, or belongs to the one that did.

Red-first on both hosts, driving that exact sequence rather than a spy: the
native test over the real seam saw the screen go `+ - + -`, and the page's
mic-control test over the port pair saw a fourth `native.audio.` verb after
B was recording. Both now end with B still holding what it took.

The mirror image is covered and now pinned at host level too: A's request
resolving late does not commit A over B, because the stale check after the
desktop start returns through `cancelStaleStart`, which cancels A's session
without touching the capture.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
This commit is contained in:
Jinwoo-H
2026-09-21 18:21:42 -04:00
parent b310033ef3
commit e17b2cf603
4 changed files with 207 additions and 28 deletions
@@ -124,7 +124,7 @@ describe('startMobileDictationDesktopSession', () => {
})
})
it('closes it even when the failure is not the current start to report', async () => {
it('leaves the capture alone when the failure is no longer the current start', async () => {
let setNewerStart = () => undefined
const harness = createStartHarness({
sendRequest: async (method) => {
@@ -139,9 +139,15 @@ describe('startMobileDictationDesktopSession', () => {
await expect(startMobileDictationDesktopSession(harness.options)).resolves.toBe(false)
// A start nobody will hear about still opened a microphone, and the screen does not care
// which generation held it.
expect(harness.rollbackRecordingStart).toHaveBeenCalledOnce()
// There is one capture seam and it carries no start identity, so a stale rejection rolling it
// back would stop whatever dictation replaced this one and hand back the screen it holds. By
// the time the generation moved, the capture was either already ended — `cancel`, `stop`, the
// unmount, a failed dictation — or belongs to a newer start.
expect(harness.rollbackRecordingStart).not.toHaveBeenCalled()
// Its own desktop session is still cancelled, which is the part that is this start's to undo.
expect(harness.sendRequest).toHaveBeenCalledWith('speech.dictation.cancel', {
dictationId: 'dictation-a'
})
})
it('does not surface a desktop-start failure after the start became stale', async () => {
@@ -56,16 +56,21 @@ export async function startMobileDictationDesktopSession(
dictationSessionStart.interpret(reply)
} catch (err) {
const wasCurrent = isCurrentStart(options)
// The hook opened the capture before this ran, and an open microphone holds the screen. No
// session started, so both go back — through the same rollback the commit failure below uses,
// because "undo the capture this start opened" is one thing and the hook owns it. Before the
// screen moved onto the mic this path leaked only an idle audio session; now it would pin the
// display until the user cancelled, retried, or the screen unmounted.
try {
options.rollbackRecordingStart()
} catch {
// Guarded for the reason the commit arm below is: a seam that throws on the way down must
// not take the desktop cancel with it, nor replace the failure the caller is about to see.
// The hook opened the capture before this ran, and an open microphone holds the screen, so a
// failure that is still this start's gives both back — through the same rollback the commit
// failure below uses, because "undo the capture this start opened" is one thing the hook owns.
//
// Only while it is still current, though: there is one capture seam and it carries no start
// identity. A stale rejection — A opened the capture, the user cancelled, B is recording —
// would end B's microphone and hand back B's screen. Past the generation, the capture was
// already ended by whatever superseded this start, or belongs to the one that did.
if (wasCurrent) {
try {
options.rollbackRecordingStart()
} catch {
// Guarded for the reason the commit arm below is: a seam that throws on the way down must
// not take the desktop cancel with it, nor replace the failure the caller is about to see.
}
}
options.clearActiveId(dictationId)
await dictationSessionCancel.request(client, { dictationId }).catch(() => undefined)
@@ -9,7 +9,11 @@
import { createElement } from 'react'
import { act, create } from 'react-test-renderer'
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { createFakeRpcClient, type FakeRpcClient } from '../mobile-web-shell/bridge-host-test-fakes'
import {
createFakeRpcClient,
type FakeRpcClient,
type SentRequest
} from '../mobile-web-shell/bridge-host-test-fakes'
const device = vi.hoisted(() => ({
/** Every engine call, in order. */
@@ -73,6 +77,25 @@ function dictation(): UseMobileDictationResult {
return current
}
/** Answers everything but the one method a case wants to keep in flight, which it collects. */
async function pumpHolding(
rpc: FakeRpcClient,
hold: string,
held: { request: SentRequest | null }
): Promise<void> {
for (let round = 0; round < 8; round += 1) {
for (const request of rpc.requests.splice(0)) {
if (request.method === hold && held.request === null) {
held.request = request
continue
}
request.resolve({ id: 'desktop', ok: true, result: {} })
}
await Promise.resolve()
await Promise.resolve()
}
}
/** Answers every forwarded request, refusing the one the case names. */
async function pump(rpc: FakeRpcClient, refuse: string): Promise<void> {
for (let round = 0; round < 8; round += 1) {
@@ -128,3 +151,86 @@ describe('a native start the desktop refuses', () => {
expect(device.calls).not.toContain('initialize')
})
})
describe('a stale native start whose refusal arrives after a newer one is recording', () => {
it('leaves the newer dictation recording, with the microphone and the screen still its own', async () => {
const rpc = createFakeRpcClient()
mount(rpc)
const first: { request: SentRequest | null } = { request: null }
// A opens the microphone and waits on the desktop.
await act(async () => {
void dictation()
.start()
.catch(() => undefined)
await pumpHolding(rpc, 'speech.dictation.start', first)
})
expect(device.screen).toEqual(['+'])
// The user gives up on A, then starts B, which takes the microphone and the screen again.
await act(async () => {
const cancelled = dictation().cancel()
await pump(rpc, 'none')
await cancelled
})
await act(async () => {
const started = dictation()
.start()
.catch(() => undefined)
await pump(rpc, 'none')
await started
})
expect(dictation().status).toBe('recording')
const screenWhileRecording = [...device.screen]
const callsWhileRecording = [...device.calls]
// Now A's request finally fails. It owns nothing: the capture and the screen are B's.
await act(async () => {
first.request?.resolve({
id: 'desktop',
ok: false,
error: { code: 'refused', message: 'no model installed' }
})
await pump(rpc, 'none')
})
expect(dictation().status).toBe('recording')
expect(device.screen).toEqual(screenWhileRecording)
expect(device.calls).toEqual(callsWhileRecording)
})
})
describe('a stale native start that succeeds after a newer one is recording', () => {
it('does not commit itself over the dictation that replaced it', async () => {
// The mirror image of the case above: A's request resolves rather than rejects. The stale
// check after the desktop start is what stops A committing, and `cancelStaleStart` cancels A's
// own session without touching the capture — which is B's.
const rpc = createFakeRpcClient()
mount(rpc)
const first: { request: SentRequest | null } = { request: null }
await act(async () => {
void dictation()
.start()
.catch(() => undefined)
await pumpHolding(rpc, 'speech.dictation.start', first)
})
await act(async () => {
const cancelled = dictation().cancel()
await pump(rpc, 'none')
await cancelled
})
await act(async () => {
const started = dictation()
.start()
.catch(() => undefined)
await pump(rpc, 'none')
await started
})
expect(dictation().status).toBe('recording')
const screenWhileRecording = [...device.screen]
const callsWhileRecording = [...device.calls]
await act(async () => {
first.request?.resolve({ id: 'desktop', ok: true, result: { started: true } })
await pump(rpc, 'none')
})
expect(dictation().status).toBe('recording')
expect(device.screen).toEqual(screenWhileRecording)
expect(device.calls).toEqual(callsWhileRecording)
})
})
@@ -80,6 +80,10 @@ function createAudioShell(): (verb: BridgeNativeVerb, params: unknown) => Promis
return (verb, params) => capture.serve(verb, params)
}
/** The hook the composer holds, for a case that has to act on a state the button does not offer:
* the cancel affordance only exists once a dictation is recording or processing. */
const composer: { dictation: ReturnType<typeof useMobileDictation> | null } = { dictation: null }
function Composer({ pair }: { pair: BridgePortPair }): ReactElement {
const dictation = useMobileDictation({
client: pair.client,
@@ -87,6 +91,7 @@ function Composer({ pair }: { pair: BridgePortPair }): ReactElement {
onTranscript: () => {},
onError: (error) => reported.push(error.message)
})
composer.dictation = dictation
return (
<MobileTerminalInputActions
canSend
@@ -114,19 +119,26 @@ function Composer({ pair }: { pair: BridgePortPair }): ReactElement {
)
}
/** What the desktop answers a forwarded request with; the default is a plain success. */
type DesktopAnswer = (method: string) => {
/** What the desktop answers a forwarded request with. `null` leaves it in flight for the case to
* settle later; the default is a plain success. */
type DesktopReply = {
id: string
ok: boolean
result?: unknown
error?: unknown
}
type DesktopAnswer = (method: string) => DesktopReply | null
const DESKTOP_OK: DesktopAnswer = () => ({ id: 'desktop', ok: true, result: {} })
/** Requests a case left unanswered, in the order the page sent them. */
const pending: { method: string; resolve: (reply: DesktopReply) => void }[] = []
type MicControl = {
readonly label: () => string
readonly tap: (answer?: DesktopAnswer) => Promise<void>
/** Drives the bridge and the desktop without pressing anything, for a case acting on the hook. */
readonly settle: (answer?: DesktopAnswer) => Promise<void>
}
/** The mic button, found by the label it carries in every state rather than by position. */
@@ -158,23 +170,34 @@ async function mount(pair: BridgePortPair): Promise<MicControl> {
if (rendered === null) {
throw new Error('nothing mounted')
}
// The tap crosses the bridge, the shell answers, and the desktop answers what was forwarded.
async function settle(answer: DesktopAnswer = DESKTOP_OK): Promise<void> {
for (let round = 0; round < 4; round += 1) {
await act(async () => {
await pair.flush()
for (const request of pair.rpc.requests.splice(0)) {
const reply = answer(request.method)
if (reply === null) {
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the fake client's resolver takes the reply shape the host would have sent.
pending.push({ method: request.method, resolve: request.resolve as never })
continue
}
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the fake client takes the reply shape the host would have sent, which is what a case builds here.
request.resolve(reply as never)
}
await pair.flush()
})
}
}
return {
label: () => String(micOf(rendered.root).props.accessibilityLabel),
settle,
tap: async (answer: DesktopAnswer = DESKTOP_OK) => {
await act(async () => {
micOf(rendered.root).props.onPress()
})
// The tap crosses the bridge, the shell answers, and the desktop answers what was forwarded.
for (let round = 0; round < 4; round += 1) {
await act(async () => {
await pair.flush()
for (const request of pair.rpc.requests.splice(0)) {
// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the fake client takes the reply shape the host would have sent, which is what a case builds here.
request.resolve(answer(request.method) as never)
}
await pair.flush()
})
}
await settle(answer)
}
}
}
@@ -182,6 +205,8 @@ async function mount(pair: BridgePortPair): Promise<MicControl> {
beforeEach(() => {
reported.length = 0
screen.length = 0
pending.length = 0
composer.dictation = null
})
afterEach(() => {
@@ -281,3 +306,40 @@ describe('the mic control on a page the shell did grant audio', () => {
).toBe(true)
})
})
describe('a stale page start whose refusal arrives after a newer one is recording', () => {
it('leaves the newer dictation recording, with the shell capture and the screen still its own', async () => {
const shell = createAudioShell()
const verbs: string[] = []
const pair = createFakeBridgePortPair({
serveNativeVerb: (verb, params) => {
verbs.push(verb)
return shell(verb, params)
}
})
const mic = await mount(pair)
// A opens the shell's microphone and waits on the desktop.
await mic.tap((method) => (method === 'speech.dictation.start' ? null : DESKTOP_OK(method)))
expect(screen).toEqual(['+'])
const first = pending.find((request) => request.method === 'speech.dictation.start')
expect(first).toBeDefined()
// The user gives up on A while it is still starting, which is a state the button has no
// affordance for, and then starts B.
await act(async () => {
void composer.dictation?.cancel()
})
await mic.settle()
await mic.tap()
expect(mic.label()).toBe('Stop voice dictation')
const verbsWhileRecording = [...verbs]
const screenWhileRecording = [...screen]
// A's request finally fails. It owns nothing: the capture and the screen are B's.
await act(async () => {
first?.resolve({ id: 'desktop', ok: false, error: { code: 'refused', message: 'no model' } })
})
await mic.settle()
expect(mic.label()).toBe('Stop voice dictation')
expect(verbs).toEqual(verbsWhileRecording)
expect(screen).toEqual(screenWhileRecording)
})
})