From e570cade3cb8132dfadf1533795da240967fc182 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 14 Aug 2026 19:20:05 -0700 Subject: [PATCH] fix(mobile): keep polling session tabs while an active terminal is pending-handle (STA-4256) (#14623) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A terminal tab published as `status: 'pending-handle'` renders the session screen's spinner. Leaving it requires a snapshot that carries the materialized handle, but a certified-live tabs stream parks `poll()` unless `hasRecoveryNeed()` says otherwise — and that predicate never considered a pending terminal. A host that mints the handle without republishing therefore stranded the pane on its spinner forever: measured live, zero further `session.tabs.list` calls over 90s while `terminal.list` kept firing every 2s. Mirrors the existing native-chat recovery-need pattern. Client-only; no wire change. --- .../app/h/[hostId]/session/[worktreeId].tsx | 5 + .../mobile-session-startup-source.test.ts | 13 ++ .../pending-terminal-handle-recovery.test.ts | 162 ++++++++++++++++++ .../pending-terminal-handle-recovery.ts | 24 +++ 4 files changed, 204 insertions(+) create mode 100644 mobile/src/session/pending-terminal-handle-recovery.test.ts create mode 100644 mobile/src/session/pending-terminal-handle-recovery.ts diff --git a/mobile/app/h/[hostId]/session/[worktreeId].tsx b/mobile/app/h/[hostId]/session/[worktreeId].tsx index 0ef8a42ee92..8bef45a6b9f 100644 --- a/mobile/app/h/[hostId]/session/[worktreeId].tsx +++ b/mobile/app/h/[hostId]/session/[worktreeId].tsx @@ -207,6 +207,7 @@ import { confirmsMirroredTabSelection, type AppliedSnapshotMarker } from '../../../../src/session/session-tab-snapshot-gate' +import { hasPendingTerminalHandleRecoveryNeed } from '../../../../src/session/pending-terminal-handle-recovery' import { createInitialSessionAutoCreateState, useInitialSessionTerminalAutoCreate, @@ -2283,6 +2284,10 @@ export default function SessionScreen() { () => closedTabTombstonesRef.current.size > 0 || pendingBrowserFocusPageIdRef.current !== null || + // Why: a pending-handle terminal only turns ready in a fresh snapshot. A live + // stream otherwise parks the poll, and a host that mints the handle without + // republishing strands the pane on its spinner forever (STA-4256). + hasPendingTerminalHandleRecoveryNeed(sessionTabsRef.current, activeSessionTabIdRef.current) || // Why: a chat-covered handle that ran out of rearms and left `terminal.list` // was reminted by a desktop graph reload. Only a fresh tab snapshot carries // the replacement handle, so force one instead of holding the composer locked. diff --git a/mobile/src/session/mobile-session-startup-source.test.ts b/mobile/src/session/mobile-session-startup-source.test.ts index 80484431c62..d8536449b33 100644 --- a/mobile/src/session/mobile-session-startup-source.test.ts +++ b/mobile/src/session/mobile-session-startup-source.test.ts @@ -186,4 +186,17 @@ describe('mobile session startup', () => { newTabActions.indexOf("label: 'Markdown Note'") ) }) + + it('counts a pending-handle active terminal as a tabs recovery need (STA-4256)', () => { + const recoveryNeed = sliceBetween( + 'const hasSessionTabsRecoveryNeed = useCallback(', + 'const getSessionTabsApplicationRevision =' + ) + + expect(recoveryNeed).toContain('hasPendingTerminalHandleRecoveryNeed(') + expect(recoveryNeed).toContain('sessionTabsRef.current') + expect(recoveryNeed).toContain('activeSessionTabIdRef.current') + // The predicate only reaches the poll loop through this hook's option. + expect(source).toContain('hasRecoveryNeed: hasSessionTabsRecoveryNeed') + }) }) diff --git a/mobile/src/session/pending-terminal-handle-recovery.test.ts b/mobile/src/session/pending-terminal-handle-recovery.test.ts new file mode 100644 index 00000000000..1945cb72114 --- /dev/null +++ b/mobile/src/session/pending-terminal-handle-recovery.test.ts @@ -0,0 +1,162 @@ +import { describe, expect, it, vi } from 'vitest' +import type { RpcClient } from '../transport/rpc-client' +import type { RpcResponse } from '../transport/types' +import type { MobileSessionTab, SessionTabsResult } from './mobile-session-route-types' +import { hasPendingTerminalHandleRecoveryNeed } from './pending-terminal-handle-recovery' +import { + MobileSessionTabsStreamHealth, + type SessionTabsApplyOutcome +} from './mobile-session-tabs-stream-health' + +const TAB_ID = 'tab-1::f47ac10b-58cc-4372-a567-0e02b2c3d479' + +function terminalTab(terminal: string | null): MobileSessionTab { + return { + type: 'terminal', + id: TAB_ID, + title: 'zsh', + parentTabId: 'tab-1', + leafId: 'f47ac10b-58cc-4372-a567-0e02b2c3d479', + status: terminal === null ? 'pending-handle' : 'ready', + terminal, + isActive: true + } +} + +function snapshot(terminal: string | null, type?: 'snapshot' | 'updated'): SessionTabsResult { + return { + worktree: 'id:repo::worktree', + publicationEpoch: 'host:1', + snapshotVersion: 1, + tabs: [terminalTab(terminal)], + activeTabId: TAB_ID, + activeTabType: 'terminal', + ...(type ? { type } : {}) + } as SessionTabsResult +} + +describe('hasPendingTerminalHandleRecoveryNeed', () => { + it('reports a need while the active terminal tab has no handle', () => { + expect(hasPendingTerminalHandleRecoveryNeed([terminalTab(null)], TAB_ID)).toBe(true) + }) + + it('clears once the host publishes the materialized handle', () => { + expect(hasPendingTerminalHandleRecoveryNeed([terminalTab('term-1')], TAB_ID)).toBe(false) + }) + + it('ignores a pending terminal the user is not looking at', () => { + const other: MobileSessionTab = { + type: 'markdown', + id: 'md-1', + title: 'NOTES.md', + filePath: '/w/NOTES.md', + relativePath: 'NOTES.md', + isDirty: false, + isActive: false, + documentVersion: '1' + } + expect(hasPendingTerminalHandleRecoveryNeed([terminalTab(null), other], 'md-1')).toBe(false) + }) + + it('reports no need with no selection or an unknown selection', () => { + expect(hasPendingTerminalHandleRecoveryNeed([terminalTab(null)], null)).toBe(false) + expect(hasPendingTerminalHandleRecoveryNeed([terminalTab(null)], 'gone')).toBe(false) + }) +}) + +/** + * STA-4256: the phone rendered its spinner forever because a `live` tabs stream + * parks `poll()`, so a host that mints the handle without republishing was never + * asked again. These drive the real controller to prove the predicate is what + * keeps `session.tabs.list` flowing, and that it stops once the handle lands. + */ +describe('pending-handle recovery through MobileSessionTabsStreamHealth', () => { + function makeHarness() { + const pending: Array<(response: RpcResponse) => void> = [] + const sendRequest = vi.fn( + () => + new Promise((resolve) => { + pending.push(resolve) + }) + ) + const client = { sendRequest, getGeneration: () => 1 } as unknown as RpcClient + let tabs: MobileSessionTab[] = [] + let activeTabId: string | null = null + const controller = new MobileSessionTabsStreamHealth({ + client, + scope: 'id:repo::worktree', + apply: (result): SessionTabsApplyOutcome => { + tabs = result.tabs + activeTabId = result.activeTabId + return { accepted: true, effectiveTabs: result.tabs } + }, + consumeAccepted: () => {}, + hasRecoveryNeed: () => hasPendingTerminalHandleRecoveryNeed(tabs, activeTabId) + }) + return { + controller, + sendRequest, + resolveNext(result: SessionTabsResult) { + const resolve = pending.shift() + expect(resolve).toBeDefined() + resolve?.({ id: 'list', ok: true, result, _meta: { runtimeId: 'runtime-1' } }) + }, + readTabs: () => tabs + } + } + + async function settle(): Promise { + await Promise.resolve() + await Promise.resolve() + await Promise.resolve() + } + + /** Subscribe, snapshot, updated — the sequence that certifies the stream `live`. */ + async function driveToLiveStream( + harness: ReturnType, + terminal: string | null + ) { + harness.controller.setReconciliationActive(true) + const subscription = harness.controller.beginSubscription() + subscription.listener(snapshot(terminal, 'snapshot')) + await settle() + harness.resolveNext(snapshot(terminal)) + await settle() + subscription.listener(snapshot(terminal, 'updated')) + await settle() + harness.sendRequest.mockClear() + expect(harness.controller.isCertified()).toBe(true) + } + + it('keeps polling session.tabs.list while the active terminal has no handle', async () => { + const harness = makeHarness() + await driveToLiveStream(harness, null) + + expect(harness.controller.poll()).not.toBeNull() + await settle() + expect(harness.sendRequest).toHaveBeenCalledWith('session.tabs.list', { + worktree: 'id:repo::worktree' + }) + }) + + it('stops polling once a fresh snapshot carries the handle', async () => { + const harness = makeHarness() + await driveToLiveStream(harness, null) + + expect(harness.controller.poll()).not.toBeNull() + await settle() + harness.resolveNext(snapshot('term-1')) + await settle() + + expect(harness.readTabs()[0]).toMatchObject({ status: 'ready', terminal: 'term-1' }) + expect(harness.controller.poll()).toBeNull() + }) + + it('parks the poll when the active terminal already has its handle', async () => { + const harness = makeHarness() + await driveToLiveStream(harness, 'term-1') + + expect(harness.controller.poll()).toBeNull() + expect(harness.sendRequest).not.toHaveBeenCalled() + }) +}) diff --git a/mobile/src/session/pending-terminal-handle-recovery.ts b/mobile/src/session/pending-terminal-handle-recovery.ts new file mode 100644 index 00000000000..c3a2a850193 --- /dev/null +++ b/mobile/src/session/pending-terminal-handle-recovery.ts @@ -0,0 +1,24 @@ +import type { MobileSessionTab } from './mobile-session-route-types' + +/** + * Whether the tab the user is looking at is a terminal the host published + * without a PTY handle (`status: 'pending-handle'`). The session screen renders + * a spinner for that tab and can only leave it when a snapshot carries the + * materialized handle — so while this holds, the tabs reconciler must keep + * asking. Without it a `live` stream parks the poll, and a host that mints the + * handle without republishing strands the pane on the spinner forever + * (STA-4256). + * + * Mirrors the route's `activePendingTerminalTab` derivation exactly, so this is + * true precisely when the spinner is on screen. + */ +export function hasPendingTerminalHandleRecoveryNeed( + tabs: readonly MobileSessionTab[], + activeTabId: string | null +): boolean { + if (activeTabId === null) { + return false + } + const active = tabs.find((tab) => tab.id === activeTabId) + return active?.type === 'terminal' && typeof active.terminal !== 'string' +}