From 6106cdaefcd2b7d94f8d59efe994037332032c24 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Fri, 4 Sep 2026 17:28:48 -0400 Subject: [PATCH] fix(mobile): fence tab activation replies to their source Ignore activation snapshots after the host, workspace, operation binding, or mounted route changes. Expo reuses session state across workspaces, so applying a late activation could repopulate the next route with the previous workspace's tabs. Cover the four invalidations and an unchanged source with React hook tests, and update the existing parity digest for the intentional guard. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --- .../mobile-session-route-parity.test.ts | 9 +- ...session-tab-activation-source-race.test.ts | 107 ++++++++++++++++++ .../use-mobile-session-tab-application.ts | 18 ++- 3 files changed, 128 insertions(+), 6 deletions(-) create mode 100644 mobile/src/session/session-tab-activation-source-race.test.ts diff --git a/mobile/src/session/mobile-session-route-parity.test.ts b/mobile/src/session/mobile-session-route-parity.test.ts index d269aee1896..87204d0aa34 100644 --- a/mobile/src/session/mobile-session-route-parity.test.ts +++ b/mobile/src/session/mobile-session-route-parity.test.ts @@ -66,11 +66,12 @@ const HOST_COMPONENT_NAMES = new Set([ 'View' ]) -const HEAD_MAIN_HOOK_SHA256 = '4fae0d13d86c343b380969797051a376a101176cfaed2f945b4fb13e86b7fc25' -const HEAD_HOOK_BINDING_SHA256 = 'f4405cb67d0879c66e87260653f68fbe8c51de6e386e75c3bdc793012cb15c4d' +// Activation source fencing adds two hooks; session-tab-activation-source-race tests the behavior. +const HEAD_MAIN_HOOK_SHA256 = '8a65c402639980ffda9132ce3dba84da8d85998f5c6b933a8c8eb09817a04783' +const HEAD_HOOK_BINDING_SHA256 = 'e43ab1ae9e6fd0eb126558207868f1da6322ba50bdeb4a6f7132e6a1f29a9474' const HEAD_CALLBACK_IDENTITY_SHA256 = '3ad3c833aa99bbfd3a4038bae70a0247192f51fb938a2fe3df86626dcfa3386e' -const HEAD_CALLBACK_BODY_SHA256 = '7da7b4dd9a184f3e583213e30e77383b5ecda1360ec53a59f16fbdbdae2d0257' +const HEAD_CALLBACK_BODY_SHA256 = '9168096f0e16c9bfe39e6c24b2365254c18f6d5a7b0611082b8723ee826fedfc' const HEAD_EFFECT_SHA256 = 'f81ef4b4794875643dd429e9dfb6cffab037feb68a334e260c0045a258c07d51' const HEAD_CONTENT_HOOK_SHA256 = 'd74431115b27c22dd38c29a510604554ca767cdd2585beaa73ec2e2dae0c5de4' const HEAD_NESTED_FUNCTION_SHA256 = @@ -479,7 +480,7 @@ describe('mobile session route extraction parity', () => { const contentBindings = CONTENT_COMPONENT_NAMES.flatMap( (name) => readHookFacts(name, definitions).bindings ) - expect(main.hooks).toHaveLength(288) + expect(main.hooks).toHaveLength(290) expect(hash(main.hooks)).toBe(HEAD_MAIN_HOOK_SHA256) expect(hash(main.bindings)).toBe(HEAD_HOOK_BINDING_SHA256) expect(main.callbacks).toHaveLength(84) diff --git a/mobile/src/session/session-tab-activation-source-race.test.ts b/mobile/src/session/session-tab-activation-source-race.test.ts new file mode 100644 index 00000000000..6b2ec7fbad1 --- /dev/null +++ b/mobile/src/session/session-tab-activation-source-race.test.ts @@ -0,0 +1,107 @@ +import { createElement } from 'react' +import { act, create } from 'react-test-renderer' +import { describe, expect, it, vi } from 'vitest' +import { useMobileSessionTabApplication } from './use-mobile-session-tab-application' +import type { MobileSessionTerminalListModel } from './use-mobile-session-terminal-list' +import type { SessionTabsResult } from './mobile-session-route-types' + +function scope() { + const ref = (current: T) => ({ current }) + return { + hostId: 'host-a', + worktreeId: 'workspace-a', + sessionTabOperations: { activate: vi.fn() }, + setWorkspaceTransportState: vi.fn(), + setTerminals: vi.fn(), + terminalsRef: ref([]), + setSessionTabs: vi.fn(), + sessionTabsRef: ref([]), + appliedSnapshotMarkerRef: ref({ epoch: null, version: -1 }), + appliedSessionTabsRevisionRef: ref(0), + closedTabTombstonesRef: ref(new Map()), + reconcileBufferedDraftsRef: ref(vi.fn()), + setTerminalsLoaded: vi.fn(), + defaultTerminalHandlesToLiveInput: vi.fn(), + setActiveHandle: vi.fn(), + setActiveSessionTabId: vi.fn(), + activeSessionTabIdRef: ref(null), + selectedSessionTabIdRef: ref(null), + markdownDocsRef: ref(new Map()), + initializedHandlesRef: ref(new Set()), + terminalDiagnosticsRef: ref({ tabsApplied: vi.fn() }), + activeHandleRef: ref(null), + activeSessionTabTypeRef: ref(null), + pendingActiveSessionTabIdRef: ref(null), + pendingActiveTerminalHandleRef: ref(null), + pendingBrowserFocusPageIdRef: ref(null), + initialSessionAutoCreateRef: ref({ sawSessionTabs: false }), + unsubscribeTerminal: vi.fn(), + subscribeToTerminal: vi.fn(), + lastKnownTerminalCountRef: ref(0), + setFileDocs: vi.fn(), + setMarkdownDocs: vi.fn(), + fileDocLifecycleRef: ref({ reconcile: vi.fn() }), + markdownDocLifecycleRef: ref({ reconcile: vi.fn() }) + } +} + +describe('session tab activation source ownership', () => { + it.each(['workspace', 'host', 'operations', 'unmount', 'unchanged'] as const)( + 'handles a delayed activation after %s changes', + async (change) => { + const initial = scope() + let resolveActivation: (snapshot: SessionTabsResult) => void = () => {} + initial.sessionTabOperations.activate.mockReturnValue( + new Promise((resolve) => { + resolveActivation = resolve + }) + ) + let actions: ReturnType | undefined + function Harness({ value }: { value: typeof initial }) { + actions = useMobileSessionTabApplication(value as unknown as MobileSessionTerminalListModel) + return null + } + let renderer: ReturnType | undefined + try { + act(() => { + renderer = create(createElement(Harness, { value: initial })) + }) + const activation = actions!.activateSessionTab('tab-a') + act(() => { + if (change === 'unmount') { + renderer?.unmount() + } else { + renderer?.update( + createElement(Harness, { + value: { + ...initial, + ...(change === 'workspace' ? { worktreeId: 'workspace-b' } : {}), + ...(change === 'host' ? { hostId: 'host-b' } : {}), + ...(change === 'operations' + ? { sessionTabOperations: { activate: vi.fn() } } + : {}) + } + }) + ) + } + }) + const snapshot = { + snapshotVersion: 1, + tabs: [], + activeTabId: null + } as unknown as SessionTabsResult + await act(async () => { + resolveActivation(snapshot) + await activation + }) + expect(initial.setSessionTabs).toHaveBeenCalledTimes(change === 'unchanged' ? 1 : 0) + expect(initial.setWorkspaceTransportState).toHaveBeenCalledTimes( + change === 'unchanged' ? 1 : 0 + ) + expect(await activation).toBe(change === 'unchanged') + } finally { + act(() => renderer?.unmount()) + } + } + ) +}) diff --git a/mobile/src/session/use-mobile-session-tab-application.ts b/mobile/src/session/use-mobile-session-tab-application.ts index dca02767a71..ea27a3b0940 100644 --- a/mobile/src/session/use-mobile-session-tab-application.ts +++ b/mobile/src/session/use-mobile-session-tab-application.ts @@ -1,4 +1,4 @@ -import { useCallback } from 'react' +import { useCallback, useLayoutEffect, useRef } from 'react' import { getTerminalRecordsFromSessionTabs, mergeTerminalRecordsByCurrentOrder, @@ -18,6 +18,7 @@ import type { MobileSessionTerminalListModel } from './use-mobile-session-termin export function useMobileSessionTabApplication(scope: MobileSessionTerminalListModel) { const { + hostId, worktreeId, sessionTabOperations, setTerminals, @@ -52,6 +53,13 @@ export function useMobileSessionTabApplication(scope: MobileSessionTerminalListM markdownDocLifecycleRef, setWorkspaceTransportState } = scope + const activationGenerationRef = useRef(0) + useLayoutEffect(() => { + activationGenerationRef.current += 1 + return () => { + activationGenerationRef.current += 1 + } + }, [hostId, worktreeId, sessionTabOperations]) const applySessionTabs = useCallback( (result: SessionTabsResult): SessionTabsApplyOutcome => { const diagnostics = terminalDiagnosticsRef.current @@ -225,7 +233,13 @@ export function useMobileSessionTabApplication(scope: MobileSessionTerminalListM if (!sessionTabOperations) { return false } - applySessionTabs(await sessionTabOperations.activate(worktreeId, tabId, leafId)) + const generation = activationGenerationRef.current + const snapshot = await sessionTabOperations.activate(worktreeId, tabId, leafId) + // Expo reuses the route; an old activation cannot publish into its replacement workspace. + if (generation !== activationGenerationRef.current) { + return false + } + applySessionTabs(snapshot) return true }, [applySessionTabs, sessionTabOperations, worktreeId]