From 0a454cc8017955e66814213390c1683e7d31ebfa Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 20 Jul 2026 17:27:20 -0700 Subject: [PATCH] fix(mobile): harden floating workspace route reuse --- .../session/use-live-worktree-name.test.ts | 52 +++++++++++++++++++ mobile/src/session/use-live-worktree-name.ts | 24 ++++++--- mobile/src/transport/client-context.test.ts | 42 ++++++++++++++- mobile/src/transport/client-context.tsx | 4 +- src/main/ipc/pty.test.ts | 17 ++++++ src/main/runtime/orca-runtime.test.ts | 12 +++-- src/main/runtime/orca-runtime.ts | 6 +-- 7 files changed, 141 insertions(+), 16 deletions(-) diff --git a/mobile/src/session/use-live-worktree-name.test.ts b/mobile/src/session/use-live-worktree-name.test.ts index 3a6e4955f12..8ac565c7684 100644 --- a/mobile/src/session/use-live-worktree-name.test.ts +++ b/mobile/src/session/use-live-worktree-name.test.ts @@ -196,4 +196,56 @@ describe('useLiveWorktreeName request volume', () => { expect(subscribe).not.toHaveBeenCalled() expect(name).toBe('Floating Workspace') }) + + it('does not render stale names while switching through the floating route', async () => { + const firstNameByWorktree = new Map() + let renderer: ReactTestRenderer | null = null + + function RouteHarness(props: { routeName?: string; worktreeId: string }): null { + const name = useLiveWorktreeName({ + client, + connState: 'connected', + routeName: props.routeName, + worktreeId: props.worktreeId + }) + if (!firstNameByWorktree.has(props.worktreeId)) { + firstNameByWorktree.set(props.worktreeId, name) + } + return null + } + + const restoreConsoleError = suppressReactTestRendererDeprecationWarning() + try { + await act(async () => { + renderer = create( + createElement(RouteHarness, { + routeName: 'Previous workspace', + worktreeId: 'repo-1::/worktree' + }) + ) + await Promise.resolve() + }) + await act(async () => { + renderer?.update( + createElement(RouteHarness, { + worktreeId: 'global-floating-terminal' + }) + ) + }) + await act(async () => { + renderer?.update( + createElement(RouteHarness, { + routeName: 'Next workspace', + worktreeId: 'repo-2::/worktree' + }) + ) + }) + } finally { + restoreConsoleError() + act(() => renderer?.unmount()) + } + + expect(firstNameByWorktree.get('global-floating-terminal')).toBe('Floating Workspace') + expect(firstNameByWorktree.get('repo-2::/worktree')).toBe('Next workspace') + }) }) diff --git a/mobile/src/session/use-live-worktree-name.ts b/mobile/src/session/use-live-worktree-name.ts index cc7c2f77e09..53ef1cf1bbf 100644 --- a/mobile/src/session/use-live-worktree-name.ts +++ b/mobile/src/session/use-live-worktree-name.ts @@ -20,11 +20,19 @@ export function useLiveWorktreeName({ client, connState, routeName, worktreeId } // Why: the floating sentinel has no worktree record, so worktree.show would // fail forever and keep the 3s fallback poll alive; its title is fixed. const isFloatingWorkspace = isFloatingWorkspaceWorktreeId(worktreeId) - const [worktreeName, setWorktreeName] = useState(() => routeName?.trim() ?? '') + const routeNameHint = routeName?.trim() ?? '' + const [worktreeName, setWorktreeName] = useState(() => ({ + worktreeId, + name: routeNameHint + })) useEffect(() => { - setWorktreeName(routeName?.trim() ?? '') - }, [routeName, worktreeId]) + setWorktreeName((current) => + current.worktreeId === worktreeId && current.name === routeNameHint + ? current + : { worktreeId, name: routeNameHint } + ) + }, [routeNameHint, worktreeId]) useFocusEffect( useCallback(() => { @@ -62,7 +70,11 @@ export function useLiveWorktreeName({ client, connState, routeName, worktreeId } ? getLiveWorktreeDisplayName([result.worktree], worktreeId) : null if (liveName) { - setWorktreeName((current) => (current === liveName ? current : liveName)) + setWorktreeName((current) => + current.worktreeId === worktreeId && current.name === liveName + ? current + : { worktreeId, name: liveName } + ) } hasSuccessfulRefresh = true if (eventStreamReady) { @@ -136,7 +148,7 @@ export function useLiveWorktreeName({ client, connState, routeName, worktreeId } ) if (isFloatingWorkspace) { - return worktreeName || FLOATING_WORKSPACE_TITLE + return FLOATING_WORKSPACE_TITLE } - return worktreeName + return worktreeName.worktreeId === worktreeId ? worktreeName.name : routeNameHint } diff --git a/mobile/src/transport/client-context.test.ts b/mobile/src/transport/client-context.test.ts index e29f4da42cc..eacb68ab0b9 100644 --- a/mobile/src/transport/client-context.test.ts +++ b/mobile/src/transport/client-context.test.ts @@ -168,7 +168,47 @@ describe('useHostClient', () => { expect(connectMock).toHaveBeenCalledTimes(2) } finally { restore() - renderer?.unmount() + act(() => renderer?.unmount()) + } + }) + + it('stays disconnected while a reused screen resolves an uncached host', async () => { + const client = makeFakeClient('connected') + connectMock.mockReturnValue(client) + loadHostsMock.mockResolvedValueOnce([HOST]).mockReturnValueOnce(new Promise(() => {})) + + let selectedHostId = HOST.id + let renderTick = 0 + const stateByRenderTick = new Map() + let renderer: ReactTestRenderer | null = null + function Probe(): null { + stateByRenderTick.set(renderTick, useHostClient(selectedHostId).state) + return null + } + + const restore = suppressReactTestRendererDeprecationWarning() + try { + await act(async () => { + renderer = create(createElement(RpcClientProvider, null, createElement(Probe))) + await Promise.resolve() + }) + expect(stateByRenderTick.get(0)).toBe('connected') + + selectedHostId = 'missing-host' + renderTick = 1 + await act(async () => { + renderer?.update(createElement(RpcClientProvider, null, createElement(Probe))) + }) + expect(stateByRenderTick.get(1)).toBe('disconnected') + + renderTick = 2 + await act(async () => { + renderer?.update(createElement(RpcClientProvider, null, createElement(Probe))) + }) + expect(stateByRenderTick.get(2)).toBe('disconnected') + } finally { + restore() + act(() => renderer?.unmount()) } }) diff --git a/mobile/src/transport/client-context.tsx b/mobile/src/transport/client-context.tsx index 00eb5e2aa29..eec5ba40469 100644 --- a/mobile/src/transport/client-context.tsx +++ b/mobile/src/transport/client-context.tsx @@ -373,9 +373,9 @@ export function useHostClient(hostId: string | undefined): { } }) const initial = ctx.acquire(hostId) + clientRef.current = initial + setState(ctx.getState(hostId)) if (initial) { - clientRef.current = initial - setState(ctx.getState(hostId)) // Why: two cached hosts can both be connected, so equal state values cannot reveal the replacement client. force((n) => n + 1) } diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index 85a84fe098b..fea2446936f 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -3623,6 +3623,23 @@ describe('registerPtyHandlers', () => { deletePtyOwnership('remote-pty') }) + it('routes runtime exact liveness without enumerating provider sessions', () => { + const provider = getLocalPtyProvider() + const hasPty = vi.spyOn(provider, 'hasPty').mockImplementation((id) => id === 'live-pty') + const listProcesses = vi.spyOn(provider, 'listProcesses') + const runtime = { setPtyController: vi.fn() } + handlers.clear() + registerPtyHandlers(mainWindow as never, runtime as never) + const controller = runtime.setPtyController.mock.calls[0]?.[0] as { + hasPty: (ptyId: string) => boolean | null + } + + expect(controller.hasPty('live-pty')).toBe(true) + expect(controller.hasPty('missing-pty')).toBe(false) + expect(hasPty).toHaveBeenCalledTimes(2) + expect(listProcesses).not.toHaveBeenCalled() + }) + it('returns unavailable runtime confirmation for unsupported or missing providers', async () => { registerSshPtyProvider('ssh-1', {} as never) setPtyOwnership('unsupported-pty', 'ssh-1') diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 9a3a6a9e5d1..804f91783b6 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -1754,14 +1754,18 @@ describe('OrcaRuntimeService', () => { }) ) const runtime = new OrcaRuntimeService({ ...runtimeStore, getRepos } as never) - const hasPty = vi.fn(() => true) - runtime.setPtyController({ + const ptyController = { + livePtyIds: new Set([floatingPtyId]), write: () => true, kill: () => true, getForegroundProcess: async () => null, - hasPty, + hasPty(this: { livePtyIds: Set }, ptyId: string) { + return this.livePtyIds.has(ptyId) + }, listProcesses - }) + } + const hasPty = vi.spyOn(ptyController, 'hasPty') + runtime.setPtyController(ptyController) const tabs = await runtime.listMobileSessionTabs(`id:${FLOATING_TERMINAL_WORKTREE_ID}`) const terminals = await runtime.listTerminals(`id:${FLOATING_TERMINAL_WORKTREE_ID}`) diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index f533a161382..d49dd782cc6 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -22018,8 +22018,8 @@ export class OrcaRuntimeService { } private refreshFloatingWorkspacePtyLiveness(): Set | null { - const hasPty = this.ptyController?.hasPty - if (!hasPty) { + const controller = this.ptyController + if (!controller?.hasPty) { return null } const knownPtyIds = new Set() @@ -22060,7 +22060,7 @@ export class OrcaRuntimeService { const liveness = new Map() try { for (const ptyId of knownPtyIds) { - const live = hasPty(ptyId) + const live = controller.hasPty(ptyId) if (live === null) { return null }