From 68ef27653e1b554f0dd04edc139a39b35bb0f2d3 Mon Sep 17 00:00:00 2001 From: Neil Date: Wed, 21 Oct 2026 01:36:39 -0700 Subject: [PATCH] fix(mobile): recover workspace capabilities after transient probe failures --- ...orktree-create-capability-recovery.test.ts | 111 +++++++++++++++++ .../tasks/worktree-create-capability.test.ts | 7 +- .../src/tasks/worktree-create-capability.ts | 112 +++++++++++++----- 3 files changed, 193 insertions(+), 37 deletions(-) create mode 100644 mobile/src/tasks/worktree-create-capability-recovery.test.ts diff --git a/mobile/src/tasks/worktree-create-capability-recovery.test.ts b/mobile/src/tasks/worktree-create-capability-recovery.test.ts new file mode 100644 index 00000000000..2a9854a8c87 --- /dev/null +++ b/mobile/src/tasks/worktree-create-capability-recovery.test.ts @@ -0,0 +1,111 @@ +import { createElement } from 'react' +import { act, create, type ReactTestRenderer } from 'react-test-renderer' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { RpcClient } from '../transport/rpc-client' +import { FakeSession } from '../transport/mobile-endpoint-supervisor-test-fakes' +import { useNewWorktreeRuntimeCapabilities } from './worktree-create-capability' + +function connection() { + let generation = 1 + const client = new FakeSession('connected') + client.getLastConnectedAt = () => generation + client.sendRequest.mockResolvedValue({ + id: 'status', + ok: true, + result: { capabilities: ['mobile.tasks.v1', 'worktree.create-idempotency.v1'] }, + _meta: { runtimeId: 'test' } + }) + return { + client, + sendRequest: client.sendRequest, + reconnect() { + generation += 1 + client.publishState('connected') + } + } +} + +describe('capability probe recovery', () => { + let renderer: ReactTestRenderer | undefined + let current: ReturnType | undefined + function Harness({ client, enabled }: { client: RpcClient; enabled: boolean }) { + current = useNewWorktreeRuntimeCapabilities(client, enabled) + return null + } + async function render(client: RpcClient, enabled = true) { + await act(async () => { + const element = createElement(Harness, { client, enabled }) + if (renderer) { + renderer.update(element) + } else { + renderer = create(element) + } + }) + } + afterEach(() => { + act(() => renderer?.unmount()) + renderer = undefined + vi.useRealTimers() + }) + + it('retries a transient failure when the create sheet reopens on the same client', async () => { + const host = connection() + host.sendRequest.mockRejectedValueOnce(new Error('offline')) + await render(host.client) + expect(current?.tasksSupported).toBe(false) + await render(host.client, false) + await render(host.client) + expect(current?.tasksSupported).toBe(true) + await expect(current?.getWorktreeCreateCutoverSupport()).resolves.toBeTruthy() + }) + + it('recovers Tasks without requiring a sheet reopen', async () => { + vi.useFakeTimers() + const host = connection() + host.sendRequest.mockRejectedValueOnce(new Error('timeout')) + await render(host.client) + await act(async () => vi.advanceTimersByTimeAsync(1_000)) + expect(current?.tasksSupported).toBe(true) + }) + + it('does not publish a retired client probe after switching to a disabled host', async () => { + const first = connection() + const second = connection() + let resolve: ((value: Awaited>) => void) | undefined + first.sendRequest.mockImplementationOnce( + () => + new Promise((done) => { + resolve = done + }) + ) + await render(first.client) + await render(second.client, false) + await act(async () => + resolve?.({ + id: 'late', + ok: true, + result: { capabilities: ['mobile.tasks.v1'] }, + _meta: { runtimeId: 'retired' } + }) + ) + expect(current?.tasksSupported).toBe(false) + expect(current?.hostPlatform).toBeNull() + expect(second.sendRequest).not.toHaveBeenCalled() + }) + + it('reprobes after reconnect while keeping confirmed old hosts conservative', async () => { + const host = connection() + host.sendRequest.mockResolvedValueOnce({ + id: 'status', + ok: true, + result: { capabilities: [] }, + _meta: { runtimeId: 'old' } + }) + await render(host.client) + await expect(current?.getWorktreeCreateCutoverSupport()).resolves.toBe(false) + expect(host.sendRequest).toHaveBeenCalledTimes(1) + await act(async () => host.reconnect()) + expect(current?.tasksSupported).toBe(true) + expect(host.sendRequest).toHaveBeenCalledTimes(2) + }) +}) diff --git a/mobile/src/tasks/worktree-create-capability.test.ts b/mobile/src/tasks/worktree-create-capability.test.ts index 78fa42be9f8..562c2032b0b 100644 --- a/mobile/src/tasks/worktree-create-capability.test.ts +++ b/mobile/src/tasks/worktree-create-capability.test.ts @@ -238,11 +238,6 @@ describe('readNewWorktreeRuntimeCapabilities', () => { }) it('fails closed when capability detection is unavailable', async () => { - await expect(readNewWorktreeRuntimeCapabilities(statusClient(['error']))).resolves.toEqual({ - tasksSupported: false, - worktreeCreateIdempotency: false, - agentLaunch: false, - hostPlatform: null - }) + await expect(readNewWorktreeRuntimeCapabilities(statusClient(['error']))).resolves.toBeNull() }) }) diff --git a/mobile/src/tasks/worktree-create-capability.ts b/mobile/src/tasks/worktree-create-capability.ts index ed354522b30..29bf92b29fa 100644 --- a/mobile/src/tasks/worktree-create-capability.ts +++ b/mobile/src/tasks/worktree-create-capability.ts @@ -27,25 +27,18 @@ export type NewWorktreeRuntimeCapabilities = { hostPlatform: NodeJS.Platform | null } -const UNSUPPORTED_CAPABILITIES: NewWorktreeRuntimeCapabilities = { - tasksSupported: false, - worktreeCreateIdempotency: false, - agentLaunch: false, - hostPlatform: null -} - // Why: status.get is safe to replay and must settle before create, independently // of slower provider probes, so ambiguous cutover retries are gated correctly. export async function readNewWorktreeRuntimeCapabilities( client: RpcClient -): Promise { +): Promise { for (let migrationRetry = 0; ; migrationRetry += 1) { try { const status = worktreeCreateCapabilityRead.interpret( await worktreeCreateCapabilityRead.request(client) ) if (!status.accepted) { - return UNSUPPORTED_CAPABILITIES + return null } const result = status.value const capabilities = result.capabilities ?? [] @@ -63,7 +56,9 @@ export async function readNewWorktreeRuntimeCapabilities( typeof advertisedIdempotency === 'object' && !Array.isArray(advertisedIdempotency) ? resolveWorktreeCreateIdempotencySupport( - (advertisedIdempotency as { dedupeTtlMs?: unknown }).dedupeTtlMs + 'dedupeTtlMs' in advertisedIdempotency + ? advertisedIdempotency.dedupeTtlMs + : undefined ) : { dedupeTtlMs: 0 } : false, @@ -71,7 +66,7 @@ export async function readNewWorktreeRuntimeCapabilities( } } catch (error) { if (!isLogicalClientCutoverError(error) || migrationRetry >= STATUS_CUTOVER_MAX_RETRIES) { - return UNSUPPORTED_CAPABILITIES + return null } } } @@ -86,49 +81,104 @@ export function useNewWorktreeRuntimeCapabilities( getWorktreeCreateCutoverSupport: () => Promise getAgentLaunchSupport: () => Promise } { - const [tasksSupported, setTasksSupported] = useState(false) - const [hostPlatform, setHostPlatform] = useState(null) + const [capabilityState, setCapabilityState] = useState<{ + client: RpcClient + capabilities: NewWorktreeRuntimeCapabilities + } | null>(null) const capabilityProbeRef = useRef<{ client: RpcClient | null - promise: Promise + generation: number | undefined + connectedAt: number | null | undefined + promise: Promise } | null>(null) - const getCapabilities = useCallback((): Promise => { - if (!capabilityProbeRef.current || capabilityProbeRef.current.client !== client) { - // Why: a queued tap can reach Create before passive effects run; lazily - // starting one shared probe keeps that path from failing open. - capabilityProbeRef.current = { + const getCapabilities = useCallback(async (): Promise => { + const generation = client?.getGeneration?.() + const connectedAt = client?.getLastConnectedAt() + let probe = capabilityProbeRef.current + if ( + !probe || + probe.client !== client || + probe.generation !== generation || + probe.connectedAt !== connectedAt + ) { + // A queued create can arrive before passive effects; share its status barrier. + probe = { client, - promise: client - ? readNewWorktreeRuntimeCapabilities(client) - : Promise.resolve(UNSUPPORTED_CAPABILITIES) + generation, + connectedAt, + promise: client ? readNewWorktreeRuntimeCapabilities(client) : Promise.resolve(null) } + capabilityProbeRef.current = probe } - return capabilityProbeRef.current.promise + const capabilities = await probe.promise + if ( + capabilityProbeRef.current !== probe || + client?.getGeneration?.() !== generation || + client?.getLastConnectedAt() !== connectedAt + ) { + return null + } + if (!capabilities && capabilityProbeRef.current === probe) { + // A failed request says nothing about what the host supports. + capabilityProbeRef.current = null + } + return capabilities }, [client]) + useEffect(() => { + if (capabilityProbeRef.current?.client !== client) { + capabilityProbeRef.current = null + } + }, [client]) + + // oxlint-disable-next-line react-doctor/effect-needs-cleanup -- Cleanup cancels the timer and fences async rearming. useEffect(() => { if (!enabled || !client) { return } let stale = false - void getCapabilities().then((capabilities) => { - if (!stale) { - setTasksSupported(capabilities.tasksSupported) - setHostPlatform(capabilities.hostPlatform) + let retry: ReturnType | undefined + let retryDelay = 1_000 + const refresh = async () => { + clearTimeout(retry) + const capabilities = await getCapabilities() + if (!stale && capabilities) { + setCapabilityState({ client, capabilities }) + retryDelay = 1_000 + } + if (!stale && !capabilityProbeRef.current && client.getState() === 'connected') { + retry = setTimeout(() => void refresh(), retryDelay) + retryDelay = Math.min(retryDelay * 2, 30_000) + } + } + const unsubscribe = client.onStateChange((state) => { + if (state === 'connected') { + capabilityProbeRef.current = null + void refresh() } }) + void refresh() return () => { stale = true + clearTimeout(retry) + unsubscribe() } - }, [client, enabled, getCapabilities, setTasksSupported]) + }, [client, enabled, getCapabilities]) const getWorktreeCreateCutoverSupport = useCallback( - () => getCapabilities().then((capabilities) => capabilities.worktreeCreateIdempotency), + () => + getCapabilities().then((capabilities) => capabilities?.worktreeCreateIdempotency ?? false), [getCapabilities] ) const getAgentLaunchSupport = useCallback( - () => getCapabilities().then((capabilities) => capabilities.agentLaunch), + () => getCapabilities().then((capabilities) => capabilities?.agentLaunch ?? false), [getCapabilities] ) - return { tasksSupported, hostPlatform, getWorktreeCreateCutoverSupport, getAgentLaunchSupport } + const capabilities = capabilityState?.client === client ? capabilityState.capabilities : null + return { + tasksSupported: capabilities?.tasksSupported ?? false, + hostPlatform: capabilities?.hostPlatform ?? null, + getWorktreeCreateCutoverSupport, + getAgentLaunchSupport + } }