From a8264bdd1ece1872ed01e7024accb2bf9987cd79 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Tue, 8 Sep 2026 00:28:17 -0400 Subject: [PATCH] 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. --- .../use-mobile-native-chat-stop.test.ts | 29 +++++++++++++++++++ .../session/use-mobile-native-chat-stop.ts | 3 ++ .../worker-terminal-takeover-report.test.ts | 18 ++++++++++++ .../worker-terminal-takeover-report.ts | 27 +++++++++++------ 4 files changed, 68 insertions(+), 9 deletions(-) diff --git a/mobile/src/session/use-mobile-native-chat-stop.test.ts b/mobile/src/session/use-mobile-native-chat-stop.test.ts index bdc405cb57d..a1ee9ab4dd9 100644 --- a/mobile/src/session/use-mobile-native-chat-stop.test.ts +++ b/mobile/src/session/use-mobile-native-chat-stop.test.ts @@ -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() + }) }) diff --git a/mobile/src/session/use-mobile-native-chat-stop.ts b/mobile/src/session/use-mobile-native-chat-stop.ts index 871d221abb2..69d2d8e73e8 100644 --- a/mobile/src/session/use-mobile-native-chat-stop.ts +++ b/mobile/src/session/use-mobile-native-chat-stop.ts @@ -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 } diff --git a/mobile/src/terminal/worker-terminal-takeover-report.test.ts b/mobile/src/terminal/worker-terminal-takeover-report.test.ts index 92aa439a698..d8e7c723b1a 100644 --- a/mobile/src/terminal/worker-terminal-takeover-report.test.ts +++ b/mobile/src/terminal/worker-terminal-takeover-report.test.ts @@ -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) +}) diff --git a/mobile/src/terminal/worker-terminal-takeover-report.ts b/mobile/src/terminal/worker-terminal-takeover-report.ts index 129b711002f..bfecfd67a36 100644 --- a/mobile/src/terminal/worker-terminal-takeover-report.ts +++ b/mobile/src/terminal/worker-terminal-takeover-report.ts @@ -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 { - const report = async () => { +async function sendTakeoverReport(client: ReportClient, terminal: string): Promise { + const report = async (): Promise => { 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((resolve) => setTimeout(resolve, REPORT_RETRY_DELAY_MS)) - await report() + return await report() } }