From 0125845d6691d1380fddb6a1be457dbcbc0e764a Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Wed, 2 Sep 2026 02:15:14 -0700 Subject: [PATCH] fix(mobile): re-hold structured chat after reconnect --- .../mobile-structured-agent-session-rpc.ts | 6 +- .../use-mobile-native-chat-controller.ts | 5 +- ...e-mobile-structured-agent-session.test.tsx | 46 ++++++++- .../use-mobile-structured-agent-session.ts | 7 +- .../use-mobile-structured-agent-state.ts | 99 +++++++++++++------ 5 files changed, 126 insertions(+), 37 deletions(-) diff --git a/mobile/src/session/mobile-structured-agent-session-rpc.ts b/mobile/src/session/mobile-structured-agent-session-rpc.ts index 2da768da542..9f6e150e698 100644 --- a/mobile/src/session/mobile-structured-agent-session-rpc.ts +++ b/mobile/src/session/mobile-structured-agent-session-rpc.ts @@ -31,11 +31,13 @@ export async function callAgentSession( client: RpcClient, method: string, params: unknown, - timeoutMs = STRUCTURED_SEND_TIMEOUT_MS + timeoutMs = STRUCTURED_SEND_TIMEOUT_MS, + options?: { failWhenDisconnected?: boolean } ): Promise { const response = await client.sendRequest(method, params, { timeoutMs, - budgetSpansConnect: true + budgetSpansConnect: true, + ...(options?.failWhenDisconnected ? { failWhenDisconnected: true } : {}) }) if (!response.ok) { throw new Error(response.error.message) diff --git a/mobile/src/session/use-mobile-native-chat-controller.ts b/mobile/src/session/use-mobile-native-chat-controller.ts index 6bb318aeef4..e67d4d0802b 100644 --- a/mobile/src/session/use-mobile-native-chat-controller.ts +++ b/mobile/src/session/use-mobile-native-chat-controller.ts @@ -93,6 +93,7 @@ export function useMobileNativeChatController(args: { client, sessionId: activeChatStructured ? activeChatSessionId : null, enabled: showNativeChat, + connected: connState === 'connected', agent: activeChatStructured ? activeChatAgent : null, onSendError }) @@ -159,8 +160,6 @@ export function useMobileNativeChatController(args: { const nativeChatTranscriptSettled = nativeChatSession.status === 'ready' || (nativeChatSession.status === 'error' && nativeChatSession.messages.length > 0) - const nativeChatAskObservable = - showNativeChat && (nativeChatDetectedAsk != null || nativeChatTranscriptSettled) const { askKey: nativeChatAskKey, showAsk: showNativeChatAsk, @@ -170,7 +169,7 @@ export function useMobileNativeChatController(args: { detectedAsk: nativeChatDetectedAsk, scopeKey: activeSessionTabId, sessionKey: activeChatSessionId, - observing: nativeChatAskObservable + observing: showNativeChat && (nativeChatDetectedAsk != null || nativeChatTranscriptSettled) }) // Every chat write gates on both: the lease proves the input floor is ours, and diff --git a/mobile/src/session/use-mobile-structured-agent-session.test.tsx b/mobile/src/session/use-mobile-structured-agent-session.test.tsx index 27257f506a6..94ad0535f3e 100644 --- a/mobile/src/session/use-mobile-structured-agent-session.test.tsx +++ b/mobile/src/session/use-mobile-structured-agent-session.test.tsx @@ -201,15 +201,18 @@ describe('useMobileStructuredAgentSession', () => { function Harness({ sessionId = 'session-1', - agent = 'codex' + agent = 'codex', + connected = true }: { sessionId?: string | null agent?: string | null + connected?: boolean }): null { hook = useMobileStructuredAgentSession({ client, sessionId, enabled: true, + connected, agent, onSendError } as never) @@ -254,6 +257,47 @@ describe('useMobileStructuredAgentSession', () => { ) }) + it('re-holds after a reconnect that outlives the host release grace', async () => { + act(() => { + renderer = create(createElement(Harness, { connected: true })) + }) + await vi.waitFor(() => + expect( + sendRequest.mock.calls.filter(([method]) => method === 'agentSession.hold') + ).toHaveLength(1) + ) + await vi.waitFor(() => expect(subscribe).toHaveBeenCalledTimes(1)) + + // A transport loss retires the connection-scoped hold; after the host's 15s grace + // it may evict the provider child. Reconnect must acquire before replaying the stream. + await act(async () => { + renderer?.update(createElement(Harness, { connected: false })) + }) + expect(unsubscribe).toHaveBeenCalledTimes(1) + await act(async () => { + renderer?.update(createElement(Harness, { connected: true })) + }) + + await vi.waitFor(() => + expect( + sendRequest.mock.calls.filter(([method]) => method === 'agentSession.hold') + ).toHaveLength(2) + ) + await vi.waitFor(() => expect(subscribe).toHaveBeenCalledTimes(2)) + const holdOrders = sendRequest.mock.calls + .map((call, index) => + call[0] === 'agentSession.hold' ? sendRequest.mock.invocationCallOrder[index] : null + ) + .filter((order): order is number => order !== null) + const subscribeOrders = subscribe.mock.invocationCallOrder + const secondHoldOrder = holdOrders[1] + const secondSubscribeOrder = subscribeOrders[1] + if (secondHoldOrder === undefined || secondSubscribeOrder === undefined) { + throw new Error('reconnect calls were not recorded') + } + expect(secondHoldOrder).toBeLessThan(secondSubscribeOrder) + }) + it('sends with the shared structured mutation envelope after the stream fence lands', async () => { act(() => { renderer = create(createElement(Harness)) diff --git a/mobile/src/session/use-mobile-structured-agent-session.ts b/mobile/src/session/use-mobile-structured-agent-session.ts index 416c7aae574..64c761b9dff 100644 --- a/mobile/src/session/use-mobile-structured-agent-session.ts +++ b/mobile/src/session/use-mobile-structured-agent-session.ts @@ -65,15 +65,18 @@ export function useMobileStructuredAgentSession(args: { client: RpcClient | null sessionId: string | null enabled: boolean + /** Live transport only; gates the connection-scoped hold, nothing else. */ + connected: boolean agent: string | null onSendError: (message: string) => void }): StructuredMobileSession { - const { agent, client, sessionId, enabled, onSendError } = args + const { agent, client, connected, sessionId, enabled, onSendError } = args const operationIdsRef = useRef(new Map()) const { state, stateRef, loadingOlder, loadEarlier } = useMobileStructuredAgentState({ client, sessionId, - enabled + enabled, + connected }) const mutate = useCallback( diff --git a/mobile/src/session/use-mobile-structured-agent-state.ts b/mobile/src/session/use-mobile-structured-agent-state.ts index 4c73f6f0c47..15d6bfe4226 100644 --- a/mobile/src/session/use-mobile-structured-agent-state.ts +++ b/mobile/src/session/use-mobile-structured-agent-state.ts @@ -27,16 +27,19 @@ export function useMobileStructuredAgentState(args: { client: RpcClient | null sessionId: string | null enabled: boolean + /** Live transport only; gates the connection-scoped hold, nothing else. */ + connected: boolean }): { state: StructuredAgentSessionState stateRef: { readonly current: StructuredAgentSessionState } loadingOlder: boolean loadEarlier: () => void } { - const { client, enabled, sessionId } = args + const { client, connected, enabled, sessionId } = args const [state, setState] = useState(EMPTY_STRUCTURED_AGENT_SESSION) const [loadingOlder, setLoadingOlder] = useState(false) const stateRef = useRef(state) + const sessionIdentityRef = useRef<{ client: RpcClient; sessionId: string } | null>(null) useLayoutEffect(() => { stateRef.current = state }, [state]) @@ -46,42 +49,80 @@ export function useMobileStructuredAgentState(args: { }, []) useEffect(() => { - if (!client || !sessionId || !enabled) { - return + const sessionChanged = + client !== null && + sessionId !== null && + (sessionIdentityRef.current?.client !== client || + sessionIdentityRef.current?.sessionId !== sessionId) + if (client && sessionId) { + sessionIdentityRef.current = { client, sessionId } } - const holderId = structuredAgentSessionHolderId('mobile-chat') - const held = callAgentSession(client, 'agentSession.hold', { - sessionId, - holderId - }).catch(() => undefined) - return () => { - void held.then(() => - callAgentSession(client, 'agentSession.release', { - sessionId, - holderId - }).catch(() => undefined) - ) - } - }, [client, enabled, sessionId]) - - useEffect(() => { if (!client || !sessionId || !enabled) { + sessionIdentityRef.current = null setState(EMPTY_STRUCTURED_AGENT_SESSION) setLoadingOlder(false) return } + if (sessionChanged) { + // Never show one structured tab's transcript under another tab. + setState(EMPTY_STRUCTURED_AGENT_SESSION) + } + if (!connected) { + // The connection-scoped hold and stream are retired in the prior cleanup; + // retain the last transcript until the transport comes back. + return + } apply({ type: 'loading' }) - const unsubscribe = client.subscribe('agentSession.subscribe', { sessionId }, (raw) => { - if (typeof raw === 'object' && raw !== null && (raw as { type?: unknown }).type === 'error') { - apply({ type: 'error', message: String((raw as { message?: unknown }).message ?? '') }) - return - } - if (isSubscribeEvent(raw)) { - apply({ type: 'event', event: raw }) - } + const holderId = structuredAgentSessionHolderId('mobile-chat') + let cancelled = false + let unsubscribe = (): void => {} + const held = callAgentSession(client, 'agentSession.hold', { + sessionId, + holderId }) - return unsubscribe - }, [apply, client, enabled, sessionId]) + void held + .then(() => { + if (cancelled) { + return + } + unsubscribe = client.subscribe('agentSession.subscribe', { sessionId }, (raw) => { + if ( + typeof raw === 'object' && + raw !== null && + (raw as { type?: unknown }).type === 'error' + ) { + apply({ type: 'error', message: String((raw as { message?: unknown }).message ?? '') }) + return + } + if (isSubscribeEvent(raw)) { + apply({ type: 'event', event: raw }) + } + }) + }) + .catch((error: unknown) => { + if (!cancelled) { + apply({ type: 'error', message: error instanceof Error ? error.message : String(error) }) + } + }) + return () => { + cancelled = true + unsubscribe() + void held + .then(() => + callAgentSession( + client, + 'agentSession.release', + { + sessionId, + holderId + }, + undefined, + { failWhenDisconnected: true } + ).catch(() => undefined) + ) + .catch(() => undefined) + } + }, [apply, client, connected, enabled, sessionId]) const loadEarlier = useCallback(() => { const current = stateRef.current