fix(mobile): a no-op takeover report does not arm the gate; Stop reports too

A key during worker startup reports before the resource is owned; caching
that zero-change reply for 30 s suppressed the report that would have fenced
the worker once it attached. Native-chat Stop is deliberate input and now
reports on an accepted Escape.
This commit is contained in:
Jinwoo-H
2026-09-08 00:28:17 -04:00
parent 23f48cadae
commit a8264bdd1e
4 changed files with 68 additions and 9 deletions
@@ -6,6 +6,12 @@ import { markRpcDeliveryUnknown } from '../transport/rpc-delivery-ambiguity'
import { MOBILE_NATIVE_CHAT_SEND_TIMEOUT_MS } from './mobile-native-chat-send'
import { useMobileNativeChatStop } from './use-mobile-native-chat-stop'
// Why mocked: the reporter is tested on its own; here Stop's escapes must be counted alone.
const reportWorkerTerminalUserInput = vi.fn()
vi.mock('../terminal/worker-terminal-takeover-report', () => ({
reportWorkerTerminalUserInput: (...args: unknown[]) => reportWorkerTerminalUserInput(...args)
}))
describe('useMobileNativeChatStop', () => {
let renderer: ReactTestRenderer | null = null
let stop: (() => void) | null = null
@@ -19,6 +25,7 @@ describe('useMobileNativeChatStop', () => {
result: { send: { accepted: true } }
})
onSendError.mockReset()
reportWorkerTerminalUserInput.mockReset()
})
afterEach(() => {
@@ -184,4 +191,26 @@ describe('useMobileNativeChatStop', () => {
expect(onSendError).not.toHaveBeenCalled()
})
it('reports the takeover once an Escape is accepted', async () => {
await render(true, 'stream-1')
act(() => stop?.())
await act(async () => vi.runAllTimersAsync())
expect(reportWorkerTerminalUserInput).toHaveBeenCalledWith(
expect.objectContaining({ sendRequest }),
'terminal-1'
)
})
it('does not report a Stop the host rejected', async () => {
sendRequest.mockResolvedValue({ ok: true, result: { send: { accepted: false } } })
await render(true, 'stream-1')
act(() => stop?.())
await act(async () => vi.runAllTimersAsync())
expect(reportWorkerTerminalUserInput).not.toHaveBeenCalled()
})
})
@@ -3,6 +3,7 @@ import type { RpcClient } from '../transport/rpc-client'
import { isRpcDeliveryUnknown } from '../transport/rpc-delivery-ambiguity'
import { isLogicalClientCutoverError } from '../transport/stable-logical-rpc-client'
import { isTerminalSendRpcAccepted } from '../terminal/terminal-send-rpc-response'
import { reportWorkerTerminalUserInput } from '../terminal/worker-terminal-takeover-report'
import { openMobileNativeChatSendBudget } from './mobile-native-chat-send'
export function useMobileNativeChatStop(args: {
@@ -111,6 +112,8 @@ export function useMobileNativeChatStop(args: {
.then((response) => {
if (isTerminalSendRpcAccepted(response)) {
sawAccepted = true
// A deliberate Stop is human input; it takes the worker over like any other key.
reportWorkerTerminalUserInput(client, handle)
} else {
sawRejected = true
}
@@ -64,3 +64,21 @@ it.each(['throw', 'rpc refusal'])(
expect(client.sendRequest).toHaveBeenCalledTimes(3)
}
)
it('a report that changed nothing does not arm the gate, so the next key reports again', async () => {
// Why: a key during worker startup lands before the resource is owned; caching that "nothing
// to fence" would suppress the report that protects the worker once it attaches.
const client = {
sendRequest: vi
.fn()
.mockResolvedValueOnce({ id: 'report', ok: true, result: { changed: 0 } })
.mockResolvedValue(success)
}
reportWorkerTerminalUserInput(client, 'term-1')
await vi.advanceTimersByTimeAsync(0)
reportWorkerTerminalUserInput(client, 'term-1')
await vi.advanceTimersByTimeAsync(0)
expect(client.sendRequest).toHaveBeenCalledTimes(2)
reportWorkerTerminalUserInput(client, 'term-1')
expect(client.sendRequest).toHaveBeenCalledTimes(2)
})
@@ -25,15 +25,23 @@ export function reportWorkerTerminalUserInput(client: ReportClient, terminal: st
}
}
reports.set(terminal, now)
void sendTakeoverReport(client, terminal).catch(() => {
if (reports.get(terminal) === now) {
reports.delete(terminal)
}
})
void sendTakeoverReport(client, terminal)
.then((changed) => {
// Why: zero rows means no worker owned this terminal yet, which is not evidence about the
// worker that may attach to it during the next 30 s; only a real transition earns the gate.
if (changed === 0 && reports.get(terminal) === now) {
reports.delete(terminal)
}
})
.catch(() => {
if (reports.get(terminal) === now) {
reports.delete(terminal)
}
})
}
async function sendTakeoverReport(client: ReportClient, terminal: string): Promise<void> {
const report = async () => {
async function sendTakeoverReport(client: ReportClient, terminal: string): Promise<number> {
const report = async (): Promise<number> => {
const response = await client.sendRequest(
'orchestration.workerTerminalUserInput',
{ terminal },
@@ -42,12 +50,13 @@ async function sendTakeoverReport(client: ReportClient, terminal: string): Promi
if (!response.ok) {
throw new Error('Worker takeover report rejected')
}
return (response.result as { changed?: number } | undefined)?.changed ?? 0
}
try {
await report()
return await report()
} catch {
await new Promise<void>((resolve) => setTimeout(resolve, REPORT_RETRY_DELAY_MS))
await report()
return await report()
}
}