diff --git a/mobile/app/h/[hostId]/session/[worktreeId].tsx b/mobile/app/h/[hostId]/session/[worktreeId].tsx index 0c781a049a9..670e2699c08 100644 --- a/mobile/app/h/[hostId]/session/[worktreeId].tsx +++ b/mobile/app/h/[hostId]/session/[worktreeId].tsx @@ -1012,6 +1012,7 @@ export default function SessionScreen() { const markdownSaveSeqRef = useRef>(new Map()) const markdownSaveInFlightRef = useRef>(new Set()) const subscribeSeqRef = useRef>(new Map()) + const chatStreamRef = useRef | null>(null) // Why: post-RPC refresh timers capture this screen and must not survive route reuse or unmount. const delayedActionTimersRef = useRef>>(new Set()) // Why: highest applyLayout seq seen per handle; drop older scrollback/resized as stale, but a >20 gap resets (fresh subscription/server restart). @@ -1299,6 +1300,7 @@ export default function SessionScreen() { unsubscribeTerminalRef.current = unsubscribeTerminal const clearTerminalCache = useCallback(() => { + chatStreamRef.current?.clearRetries() terminalUnsubsRef.current.forEach((unsub) => unsub()) clearNativeChatInputLease() terminalUnsubsRef.current.clear() @@ -1334,21 +1336,21 @@ export default function SessionScreen() { ) const subscribeToTerminal = useCallback( - (handle: string) => { + (handle: string): boolean | void => { const diagnostics = terminalDiagnosticsRef.current const logSkippedGate = (reason: string) => diagnostics.streamSkipped(handle, reason, handle === activeHandleRef.current) if (!client) { logSkippedGate('no-client') - return + return false } if (terminalUnsubsRef.current.has(handle)) { logSkippedGate('already-subscribed') - return + return false } if (subscribingHandlesRef.current.has(handle)) { logSkippedGate('subscribe-in-flight') - return + return false } const covered = nativeChatTerminalStream.isTerminalCoveredByNativeChat( showNativeChatRef.current, @@ -1359,43 +1361,41 @@ export default function SessionScreen() { if (!covered) { if (!getTerminalRef(handle)) { logSkippedGate('no-webview-ref') - return + return false } if (!webReadyHandlesRef.current.has(handle)) { logSkippedGate('webview-not-ready') - return + return false } } subscribingHandlesRef.current.add(handle) const seq = (subscribeSeqRef.current.get(handle) ?? 0) + 1 subscribeSeqRef.current.set(handle, seq) - diagnostics.streamArmed(handle, seq, viewportRef.current) - - // Why: viewport is embedded in the subscribe params so the server auto-fits before serializing scrollback (no focus→safeFit race). + diagnostics.streamArmed(handle, seq, covered ? null : viewportRef.current, covered) + const terminateStream = () => + chatStreamRef.current?.terminateStream(handle, unsubscribeTerminalRef.current) const unsub = subscribeMobileTerminalSafely( client, - { + nativeChatTerminalStream.buildMobileNativeChatTerminalSubscribeParams({ terminal: handle, - client: { id: deviceTokenRef.current!, type: 'mobile' as const }, - viewport: nativeChatTerminalStream.mobileNativeChatSubscribeViewport( - covered, - viewportRef.current - ), - capabilities: nativeChatTerminalStream.mobileNativeChatTerminalCapabilities(covered) - }, + clientId: deviceTokenRef.current!, + covered, + viewport: viewportRef.current + }), (result) => { if (subscribeSeqRef.current.get(handle) !== seq) { return } const data = result as Record - diagnostics.firstStreamEvent(handle, seq, data.type) + diagnostics.firstStreamEvent(handle, seq, data) if (data.type === 'end' || data.type === 'error') { - unsubscribeTerminalRef.current(handle) + terminateStream() return } - if (data.type === 'subscribed') { + if (nativeChatTerminalStream.isMobileNativeChatLeaseReady(covered, data)) { markNativeChatInputLeaseReady(handle) + chatStreamRef.current?.notifyStreamReady(handle) return } // Why: keep the subscription as the input-floor lease but don't mutate covered xterm state; return-to-terminal resubscribes. @@ -1538,7 +1538,12 @@ export default function SessionScreen() { scheduleDelayedAction(() => getTerminalRef(handle)?.resetZoom(), 200) } }, - () => unsubscribeTerminalRef.current(handle) + () => { + if (subscribeSeqRef.current.get(handle) !== seq) { + return + } + terminateStream() + } ) if (subscribeSeqRef.current.get(handle) === seq) { @@ -1551,7 +1556,7 @@ export default function SessionScreen() { [client, getTerminalRef, markNativeChatInputLeaseReady, scheduleDelayedAction] ) - const notifyTerminalWebReady = useMobileNativeChatTerminalStream({ + useMobileNativeChatTerminalStream({ showNativeChat, activeHandle, activeTabType: activeSessionTab?.type ?? null, @@ -1560,9 +1565,9 @@ export default function SessionScreen() { webReadyRef: webReadyHandlesRef, initializedRef: initializedHandlesRef, subscribe: subscribeToTerminal, - unsubscribe: unsubscribeTerminal + unsubscribe: unsubscribeTerminal, + controllerRef: chatStreamRef }) - // Why: server does the resize and emits 'resized' on the existing subscription — no client-side state tracking needed. const toggleInFlightRef = useRef>(new Set()) const toggleDisplayMode = useCallback( @@ -2918,7 +2923,7 @@ export default function SessionScreen() { (handle: string) => { const wasAlreadyReady = webReadyHandlesRef.current.has(handle) webReadyHandlesRef.current.add(handle) - notifyTerminalWebReady(handle, wasAlreadyReady) + chatStreamRef.current?.notifyWebReady(handle, wasAlreadyReady) terminalDiagnosticsRef.current.webViewReady( handle, wasAlreadyReady, @@ -2946,7 +2951,7 @@ export default function SessionScreen() { })() } }, - [measureViewportOnce, notifyTerminalWebReady, subscribeToTerminal, unsubscribeTerminal] + [measureViewportOnce, subscribeToTerminal, unsubscribeTerminal] ) useEffect(() => { diff --git a/mobile/src/session/MobileNativeChatComposer.test.ts b/mobile/src/session/MobileNativeChatComposer.test.ts index ef2c1012603..f00fb521f21 100644 --- a/mobile/src/session/MobileNativeChatComposer.test.ts +++ b/mobile/src/session/MobileNativeChatComposer.test.ts @@ -40,20 +40,28 @@ function suppressRendererWarning(): () => void { describe('MobileNativeChatComposer', () => { let renderer: ReactTestRenderer | null = null + let previousActEnvironment: boolean | undefined beforeEach(() => { + previousActEnvironment = globalThis.IS_REACT_ACT_ENVIRONMENT globalThis.IS_REACT_ACT_ENVIRONMENT = true }) afterEach(() => { act(() => renderer?.unmount()) renderer = null + globalThis.IS_REACT_ACT_ENVIRONMENT = previousActEnvironment }) async function render( onSend: (text: string) => Promise, onChangeText: () => void, - isAttaching = false + options: { + isAttaching?: boolean + sendDisabled?: boolean + onAttachImage?: () => void + onMicPress?: () => void + } = {} ) { const restore = suppressRendererWarning() try { @@ -63,7 +71,7 @@ describe('MobileNativeChatComposer', () => { value: ' hello ', onChangeText, onSend, - isAttaching + ...options }) ) }) @@ -105,13 +113,38 @@ describe('MobileNativeChatComposer', () => { it('disables send while an attachment path is still being injected', async () => { const onSend = vi.fn().mockResolvedValue(true) - await render(onSend, vi.fn(), true) + await render(onSend, vi.fn(), { isAttaching: true }) expect(sendButton().props).toMatchObject({ disabled: true }) await act(async () => sendButton().props.onPress()) expect(onSend).not.toHaveBeenCalled() }) + it('disables only Send while the terminal lease is pending', async () => { + const onSend = vi.fn().mockResolvedValue(true) + await render(onSend, vi.fn(), { + sendDisabled: true, + onAttachImage: vi.fn(), + onMicPress: vi.fn() + }) + + expect(sendButton().props).toMatchObject({ disabled: true }) + expect(renderer!.root.findByType('TextInput').props.editable).toBeUndefined() + expect( + renderer!.root.find( + (node) => node.type === 'Pressable' && node.props.accessibilityLabel === 'Attach image' + ).props.disabled + ).toBe(false) + expect( + renderer!.root.find( + (node) => node.type === 'Pressable' && node.props.accessibilityLabel === 'Dictate' + ).props.disabled + ).toBeUndefined() + + await act(async () => sendButton().props.onPress()) + expect(onSend).not.toHaveBeenCalled() + }) + it('moves the caret to the insert point after an autocomplete pick, then releases control', async () => { const restore = suppressRendererWarning() try { diff --git a/mobile/src/session/MobileNativeChatComposer.tsx b/mobile/src/session/MobileNativeChatComposer.tsx index ba02fe3d44b..5728d89a971 100644 --- a/mobile/src/session/MobileNativeChatComposer.tsx +++ b/mobile/src/session/MobileNativeChatComposer.tsx @@ -44,7 +44,7 @@ type Props = { dictationMode?: 'toggle' | 'hold' onMicPressIn?: () => void onMicPressOut?: () => void - disabled?: boolean + sendDisabled?: boolean placeholder?: string filePaths?: string[] onNeedFiles?: (query: string) => void @@ -61,7 +61,7 @@ export function MobileNativeChatComposer({ dictationMode = 'toggle', onMicPressIn, onMicPressOut, - disabled = false, + sendDisabled = false, placeholder = 'Message, @files, /commands', filePaths = NO_FILE_PATHS, onNeedFiles @@ -76,7 +76,7 @@ export function MobileNativeChatComposer({ const sendingRef = useRef(false) const [sending, setSending] = useState(false) const trimmed = value.trim() - const canSend = trimmed.length > 0 && !disabled && !sending && !isAttaching + const canSend = trimmed.length > 0 && !sendDisabled && !sending && !isAttaching const trigger = useMemo(() => detectAutocompleteTrigger(value, cursor), [value, cursor]) const suggestions = useMemo(() => { @@ -151,7 +151,7 @@ export function MobileNativeChatComposer({ accessibilityLabel="Attach image" style={({ pressed }) => [styles.iconButton, pressed && styles.pressed]} onPress={onAttachImage} - disabled={isAttaching || disabled} + disabled={isAttaching} > {isAttaching ? ( @@ -174,7 +174,6 @@ export function MobileNativeChatComposer({ placeholderTextColor={colors.textMuted} selectionColor={colors.accentBlue} multiline - editable={!disabled} textAlignVertical="top" /> {onMicPress ? ( @@ -185,7 +184,6 @@ export function MobileNativeChatComposer({ onPress={dictationMode === 'hold' ? undefined : onMicPress} onPressIn={dictationMode === 'hold' ? onMicPressIn : undefined} onPressOut={dictationMode === 'hold' ? onMicPressOut : undefined} - disabled={disabled} > {micActive ? ( ({ + ActivityIndicator: 'ActivityIndicator', + FlatList: 'FlatList', + Pressable: 'Pressable', + Text: 'Text', + View: 'View' +})) + +vi.mock('react-native-safe-area-context', () => ({ + useSafeAreaInsets: () => ({ bottom: 0 }) +})) + +vi.mock('react-native-gesture-handler', () => ({ + GestureDetector: 'GestureDetector', + GestureHandlerRootView: 'GestureHandlerRootView' +})) + +vi.mock('lucide-react-native', () => ({ + ArrowDown: 'ArrowDown', + ChevronsDownUp: 'ChevronsDownUp', + ChevronsUpDown: 'ChevronsUpDown', + Square: 'Square' +})) + +vi.mock('./mobile-native-chat-view-styles', () => ({ styles: {} })) +vi.mock('./mobile-native-chat-render-data', () => ({ + buildMobileNativeChatTransientData: () => ({ data: [] }), + foldMobileNativeChatMessages: () => [], + mobileNativeChatEmptyState: () => null +})) +vi.mock('./use-mobile-native-chat-ask-dismiss', () => ({ + useMobileNativeChatAskDismiss: () => ({ + askKey: null, + showAsk: false, + dismissAsk: vi.fn() + }) +})) +vi.mock('./use-mobile-native-chat-pinch-gesture', () => ({ + useMobileNativeChatPinchGesture: () => ({ fontScale: 1, pinchGesture: {} }) +})) +vi.mock('./MobileAgentWorkingIndicator', () => ({ + MobileAgentWorkingIndicator: 'MobileAgentWorkingIndicator' +})) +vi.mock('./MobileNativeChatComposer', () => ({ + MobileNativeChatComposer: 'MobileNativeChatComposer' +})) +vi.mock('./MobileNativeChatMessage', () => ({ + MobileNativeChatMessage: 'MobileNativeChatMessage' +})) +vi.mock('./MobileNativeChatAsk', () => ({ MobileNativeChatAsk: 'MobileNativeChatAsk' })) +vi.mock('./MobileNativeChatPermission', () => ({ + MobileNativeChatPermission: 'MobileNativeChatPermission' +})) +vi.mock('./MobileNativeChatQuestion', () => ({ + MobileNativeChatQuestion: 'MobileNativeChatQuestion' +})) + +describe('MobileNativeChatView', () => { + let renderer: ReactTestRenderer | null = null + let previousActEnvironment: boolean | undefined + + beforeEach(() => { + previousActEnvironment = globalThis.IS_REACT_ACT_ENVIRONMENT + globalThis.IS_REACT_ACT_ENVIRONMENT = true + vi.useFakeTimers() + }) + + afterEach(() => { + act(() => renderer?.unmount()) + renderer = null + vi.useRealTimers() + globalThis.IS_REACT_ACT_ENVIRONMENT = previousActEnvironment + }) + + function composerProps(): { sendDisabled: boolean; placeholder: string } { + return renderer!.root.findByType('MobileNativeChatComposer').props as { + sendDisabled: boolean + placeholder: string + } + } + + function suppressRendererWarning(): () => void { + const original = console.error + const error = vi.spyOn(console, 'error').mockImplementation((...args) => { + if (typeof args[0] === 'string' && args[0].includes('react-test-renderer is deprecated')) { + return + } + original(...args) + }) + return () => error.mockRestore() + } + + it('disables sending immediately while delaying only the waiting copy', async () => { + const restore = suppressRendererWarning() + try { + await act(async () => { + renderer = create( + createElement(MobileNativeChatView, { + messages: [], + status: 'ready', + onSend: vi.fn().mockResolvedValue(true), + pending: [], + composerText: 'hello', + onComposerTextChange: vi.fn(), + inputLockReason: 'waiting' + }) + ) + }) + + expect(composerProps()).toMatchObject({ + sendDisabled: true, + placeholder: 'Message, @files, /commands' + }) + + await act(async () => vi.advanceTimersByTime(600)) + expect(composerProps()).toMatchObject({ + sendDisabled: true, + placeholder: 'Waiting for terminal…' + }) + + await act(async () => { + renderer?.update( + createElement(MobileNativeChatView, { + messages: [], + status: 'ready', + onSend: vi.fn().mockResolvedValue(true), + pending: [], + composerText: 'hello', + onComposerTextChange: vi.fn(), + inputLockReason: null + }) + ) + }) + expect(composerProps()).toMatchObject({ + sendDisabled: false, + placeholder: 'Message, @files, /commands' + }) + } finally { + restore() + } + }) + + it('cancels waiting copy when the lease unlocks before the delay', async () => { + const restore = suppressRendererWarning() + try { + await act(async () => { + renderer = create( + createElement(MobileNativeChatView, { + messages: [], + status: 'ready', + onSend: vi.fn().mockResolvedValue(true), + pending: [], + composerText: 'hello', + onComposerTextChange: vi.fn(), + inputLockReason: 'waiting' + }) + ) + }) + await act(async () => vi.advanceTimersByTime(300)) + await act(async () => { + renderer?.update( + createElement(MobileNativeChatView, { + messages: [], + status: 'ready', + onSend: vi.fn().mockResolvedValue(true), + pending: [], + composerText: 'hello', + onComposerTextChange: vi.fn(), + inputLockReason: null + }) + ) + }) + await act(async () => vi.advanceTimersByTime(600)) + + expect(composerProps()).toMatchObject({ + sendDisabled: false, + placeholder: 'Message, @files, /commands' + }) + } finally { + restore() + } + }) + + it('updates an already-visible lock reason without another copy delay', async () => { + const restore = suppressRendererWarning() + try { + await act(async () => { + renderer = create( + createElement(MobileNativeChatView, { + messages: [], + status: 'ready', + onSend: vi.fn().mockResolvedValue(true), + pending: [], + composerText: 'hello', + onComposerTextChange: vi.fn(), + inputLockReason: 'waiting' + }) + ) + }) + await act(async () => vi.advanceTimersByTime(600)) + await act(async () => { + renderer?.update( + createElement(MobileNativeChatView, { + messages: [], + status: 'ready', + onSend: vi.fn().mockResolvedValue(true), + pending: [], + composerText: 'hello', + onComposerTextChange: vi.fn(), + inputLockReason: 'disconnected' + }) + ) + }) + + expect(composerProps()).toMatchObject({ + sendDisabled: true, + placeholder: 'Reconnecting…' + }) + } finally { + restore() + } + }) +}) diff --git a/mobile/src/session/MobileNativeChatView.tsx b/mobile/src/session/MobileNativeChatView.tsx index 1cfe37e5d4d..34d553bb3b2 100644 --- a/mobile/src/session/MobileNativeChatView.tsx +++ b/mobile/src/session/MobileNativeChatView.tsx @@ -235,9 +235,8 @@ export function MobileNativeChatView({ const emptyState = mobileNativeChatEmptyState(status, agent ?? null, error) const showLoading = status === 'loading' && messages.length === 0 - // Composer-lock flicker guard: on a remote link, brief connState blips or lease - // hand-offs would otherwise toggle the lock placeholder on and off. Only surface - // a lock once it has held ~600ms; drop it instantly so unlocking stays snappy. + // Why: gate Send immediately, but keep draft editing available while the + // acknowledged lease catches up; delay only the placeholder swap. const rawLockReason = inputLockReason ?? null const [lockHeld, setLockHeld] = useState(false) useEffect(() => { @@ -409,7 +408,7 @@ export function MobileNativeChatView({ dictationMode={dictationMode} onMicPressIn={onMicPressIn} onMicPressOut={onMicPressOut} - disabled={lockReason !== null} + sendDisabled={rawLockReason !== null} placeholder={ lockReason === 'disconnected' ? 'Reconnecting…' diff --git a/mobile/src/session/mobile-native-chat-terminal-stream.test.ts b/mobile/src/session/mobile-native-chat-terminal-stream.test.ts index 9b751c5af75..d61e9018a76 100644 --- a/mobile/src/session/mobile-native-chat-terminal-stream.test.ts +++ b/mobile/src/session/mobile-native-chat-terminal-stream.test.ts @@ -1,8 +1,11 @@ import { describe, expect, it } from 'vitest' import { + buildMobileNativeChatTerminalSubscribeParams, + isMobileNativeChatLeaseReady, isTerminalCoveredByNativeChat, mobileNativeChatSubscribeViewport, mobileNativeChatTerminalCapabilities, + mobileNativeChatTerminalRetryDelay, resolveMobileNativeChatTerminalStreamAction } from './mobile-native-chat-terminal-stream' @@ -33,6 +36,31 @@ describe('mobile native-chat terminal stream lifecycle', () => { mobileInputLeaseOnly: 1 }) expect(mobileNativeChatTerminalCapabilities(false)).toEqual({ terminalBinaryStream: 1 }) + expect( + buildMobileNativeChatTerminalSubscribeParams({ + terminal: 'pty-1', + clientId: 'phone-1', + covered: true, + viewport: { cols: 48, rows: 20 } + }) + ).toEqual({ + terminal: 'pty-1', + client: { id: 'phone-1', type: 'mobile' }, + capabilities: { terminalBinaryStream: 1, mobileInputLeaseOnly: 1 } + }) + expect( + buildMobileNativeChatTerminalSubscribeParams({ + terminal: 'pty-1', + clientId: 'phone-1', + covered: false, + viewport: { cols: 48, rows: 20 } + }) + ).toEqual({ + terminal: 'pty-1', + client: { id: 'phone-1', type: 'mobile' }, + viewport: { cols: 48, rows: 20 }, + capabilities: { terminalBinaryStream: 1 } + }) }) it('omits the viewport from a covered lease subscribe so the host keeps desktop dims', () => { @@ -97,4 +125,19 @@ describe('mobile native-chat terminal stream lifecycle', () => { 'none' ) }) + + it('backs off covered-chat retries while keeping recovery bounded', () => { + expect([0, 1, 2, 3, 4, 5].map(mobileNativeChatTerminalRetryDelay)).toEqual([ + 250, 500, 1_000, 2_000, 4_000, 4_000 + ]) + }) + + it('accepts only real or legacy-compatible lease acknowledgements', () => { + expect(isMobileNativeChatLeaseReady(true, { type: 'subscribed', leaseReady: true })).toBe(true) + expect(isMobileNativeChatLeaseReady(true, { type: 'subscribed' })).toBe(true) + expect(isMobileNativeChatLeaseReady(true, { type: 'terminal-unavailable' })).toBe(false) + expect(isMobileNativeChatLeaseReady(true, { type: 'subscribed', leaseReady: false })).toBe( + false + ) + }) }) diff --git a/mobile/src/session/mobile-native-chat-terminal-stream.ts b/mobile/src/session/mobile-native-chat-terminal-stream.ts index d96ced2a6f6..034340564f7 100644 --- a/mobile/src/session/mobile-native-chat-terminal-stream.ts +++ b/mobile/src/session/mobile-native-chat-terminal-stream.ts @@ -1,5 +1,15 @@ export type MobileNativeChatTerminalStreamAction = 'pause' | 'resume' | 'none' +const MOBILE_NATIVE_CHAT_TERMINAL_RETRY_BASE_MS = 250 +const MOBILE_NATIVE_CHAT_TERMINAL_RETRY_MAX_MS = 4_000 + +export function mobileNativeChatTerminalRetryDelay(attempt: number): number { + return Math.min( + MOBILE_NATIVE_CHAT_TERMINAL_RETRY_BASE_MS * 2 ** Math.max(0, attempt), + MOBILE_NATIVE_CHAT_TERMINAL_RETRY_MAX_MS + ) +} + /** Decides whether the active mobile terminal stream should run while native chat * covers its WebView. Resume is allowed only once the mounted WebView is ready. */ export function resolveMobileNativeChatTerminalStreamAction(args: { @@ -27,6 +37,18 @@ export function isTerminalCoveredByNativeChat( return showNativeChat && activeHandle === handle } +export function isMobileNativeChatLeaseReady( + covered: boolean, + event: Readonly> +): boolean { + if (event.type !== 'subscribed') { + return false + } + // Why: old hosts predate leaseReady but their lease-only subscribed event is + // still authoritative; new hosts explicitly distinguish it from PTY timeout. + return !covered || event.leaseReady === true || event.leaseReady === undefined +} + export function mobileNativeChatTerminalCapabilities(covered: boolean): { terminalBinaryStream: 1 mobileInputLeaseOnly?: 1 @@ -43,3 +65,25 @@ export function mobileNativeChatSubscribeViewport( ): { cols: number; rows: number } | undefined { return covered ? undefined : (viewport ?? undefined) } + +export function buildMobileNativeChatTerminalSubscribeParams(args: { + terminal: string + clientId: string + covered: boolean + viewport: { cols: number; rows: number } | null +}): { + terminal: string + client: { id: string; type: 'mobile' } + viewport?: { cols: number; rows: number } + capabilities: { terminalBinaryStream: 1; mobileInputLeaseOnly?: 1 } +} { + // Why: chat needs only the input lease; awaiting a hidden terminal resize can + // otherwise hold the acknowledgement behind SSH/provider latency. + const viewport = mobileNativeChatSubscribeViewport(args.covered, args.viewport) + return { + terminal: args.terminal, + client: { id: args.clientId, type: 'mobile' }, + ...(viewport ? { viewport } : {}), + capabilities: mobileNativeChatTerminalCapabilities(args.covered) + } +} diff --git a/mobile/src/session/mobile-native-chat-terminal-subscribe-source.test.ts b/mobile/src/session/mobile-native-chat-terminal-subscribe-source.test.ts new file mode 100644 index 00000000000..5f116c1ec72 --- /dev/null +++ b/mobile/src/session/mobile-native-chat-terminal-subscribe-source.test.ts @@ -0,0 +1,23 @@ +import { readFileSync } from 'node:fs' +import { describe, expect, it } from 'vitest' + +const source = readFileSync( + new URL('../../app/h/[hostId]/session/[worktreeId].tsx', import.meta.url), + 'utf8' +) + +describe('mobile native-chat terminal subscribe call site', () => { + it('routes covered and visible subscriptions through the tested params builder', () => { + const start = source.indexOf('const subscribeToTerminal = useCallback(') + const end = source.indexOf('const toggleInFlightRef =', start) + expect(start).toBeGreaterThanOrEqual(0) + expect(end).toBeGreaterThan(start) + const subscribeSource = source.slice(start, end) + + expect(subscribeSource).toContain( + 'nativeChatTerminalStream.buildMobileNativeChatTerminalSubscribeParams({' + ) + expect(subscribeSource).toContain('covered,\n viewport: viewportRef.current') + expect(subscribeSource).not.toContain('capabilities: { terminalBinaryStream: 1') + }) +}) diff --git a/mobile/src/session/mobile-terminal-diagnostics.test.ts b/mobile/src/session/mobile-terminal-diagnostics.test.ts index 831ad4bfccc..a60ece4dc3d 100644 --- a/mobile/src/session/mobile-terminal-diagnostics.test.ts +++ b/mobile/src/session/mobile-terminal-diagnostics.test.ts @@ -36,11 +36,42 @@ describe('mobile terminal diagnostics', () => { const log = vi.spyOn(console, 'log').mockImplementation(() => {}) const diagnostics = new MobileTerminalDiagnostics() - diagnostics.firstStreamEvent('terminal-1', 1, 'subscribed') + diagnostics.firstStreamEvent('terminal-1', 1, { type: 'subscribed' }) diagnostics.terminalUnsubscribed('terminal-1') - diagnostics.firstStreamEvent('terminal-1', 1, 'subscribed') + diagnostics.firstStreamEvent('terminal-1', 1, { type: 'subscribed' }) expect(log).toHaveBeenCalledTimes(2) log.mockRestore() }) + + it('measures lease-only acknowledgement latency', () => { + const log = vi.spyOn(console, 'log').mockImplementation(() => {}) + vi.useFakeTimers() + try { + vi.setSystemTime(1_000) + const diagnostics = new MobileTerminalDiagnostics() + + diagnostics.streamArmed('terminal-1', 3, null, true) + vi.setSystemTime(1_275) + diagnostics.firstStreamEvent('terminal-1', 3, { + type: 'subscribed', + readinessTiming: { serverTotalMs: 200, ptyWaitMs: 180, leaseRegisterMs: 5 } + }) + + expect(log).toHaveBeenLastCalledWith('[terminal-diagnostic]', 'stream-first-event', { + handle: 'rminal-1', + seq: 3, + type: 'subscribed', + leaseOnly: true, + waitMs: 275, + serverTotalMs: 200, + serverPtyWaitMs: 180, + serverLeaseRegisterMs: 5, + estimatedTransportMs: 75 + }) + } finally { + vi.useRealTimers() + log.mockRestore() + } + }) }) diff --git a/mobile/src/session/mobile-terminal-diagnostics.ts b/mobile/src/session/mobile-terminal-diagnostics.ts index 8695928ed03..cbc3cb59925 100644 --- a/mobile/src/session/mobile-terminal-diagnostics.ts +++ b/mobile/src/session/mobile-terminal-diagnostics.ts @@ -21,6 +21,14 @@ type DiagnosticTabsSnapshot = { type DiagnosticDimensions = { readonly cols: number; readonly rows: number } | null | undefined +function diagnosticDuration( + source: Readonly> | undefined, + key: string +): number | undefined { + const value = source?.[key] + return typeof value === 'number' && Number.isFinite(value) && value >= 0 ? value : undefined +} + // Why: full runtime identifiers make shared logs unnecessarily sensitive; the // suffix is enough to correlate lifecycle events within one reproduction. export function shortenMobileTerminalDiagnosticId(value: string | null | undefined): string | null { @@ -53,6 +61,10 @@ export function logMobileTerminalDiagnostic( export class MobileTerminalDiagnostics { private readonly streamGateByHandle = new Map() private readonly firstStreamEventSeqByHandle = new Map() + private readonly streamArmedAtByHandle = new Map< + string, + { seq: number; armedAt: number; leaseOnly: boolean } + >() private lastFetchedTabsSignature: string | null = null private lastAppliedTabsSignature: string | null = null private lastTabsFetchStartAt = 0 @@ -61,6 +73,7 @@ export class MobileTerminalDiagnostics { clearTerminalCache(): void { this.streamGateByHandle.clear() this.firstStreamEventSeqByHandle.clear() + this.streamArmedAtByHandle.clear() } resetRoute(): void { @@ -74,6 +87,7 @@ export class MobileTerminalDiagnostics { terminalUnsubscribed(handle: string): void { this.streamGateByHandle.delete(handle) this.firstStreamEventSeqByHandle.delete(handle) + this.streamArmedAtByHandle.delete(handle) } viewportMeasured(handle: string, dims: DiagnosticDimensions, frameHeight: number): void { @@ -98,26 +112,51 @@ export class MobileTerminalDiagnostics { }) } - streamArmed(handle: string, seq: number, viewport: DiagnosticDimensions): void { + streamArmed( + handle: string, + seq: number, + viewport: DiagnosticDimensions, + leaseOnly = false + ): void { this.streamGateByHandle.delete(handle) + this.streamArmedAtByHandle.set(handle, { seq, armedAt: Date.now(), leaseOnly }) logMobileTerminalDiagnostic('stream-armed', { handle: shortenMobileTerminalDiagnosticId(handle), seq, + leaseOnly, hasViewport: viewport != null, viewportCols: viewport?.cols, viewportRows: viewport?.rows }) } - firstStreamEvent(handle: string, seq: number, type: unknown): void { + firstStreamEvent(handle: string, seq: number, event: unknown): void { if (this.firstStreamEventSeqByHandle.get(handle) === seq) { return } this.firstStreamEventSeqByHandle.set(handle, seq) + const armed = this.streamArmedAtByHandle.get(handle) + const data = + event && typeof event === 'object' ? (event as Readonly>) : undefined + const timing = + data?.readinessTiming && typeof data.readinessTiming === 'object' + ? (data.readinessTiming as Readonly>) + : undefined + const waitMs = armed?.seq === seq ? Math.max(0, Date.now() - armed.armedAt) : undefined + const serverTotalMs = diagnosticDuration(timing, 'serverTotalMs') logMobileTerminalDiagnostic('stream-first-event', { handle: shortenMobileTerminalDiagnosticId(handle), seq, - type: typeof type === 'string' ? type : 'unknown' + type: typeof data?.type === 'string' ? data.type : 'unknown', + leaseOnly: armed?.seq === seq ? armed.leaseOnly : undefined, + waitMs, + serverTotalMs, + serverPtyWaitMs: diagnosticDuration(timing, 'ptyWaitMs'), + serverLeaseRegisterMs: diagnosticDuration(timing, 'leaseRegisterMs'), + estimatedTransportMs: + waitMs !== undefined && serverTotalMs !== undefined + ? Math.max(0, waitMs - serverTotalMs) + : undefined }) } diff --git a/mobile/src/session/use-mobile-native-chat-terminal-stream.test.ts b/mobile/src/session/use-mobile-native-chat-terminal-stream.test.ts index 92320b9ff86..b7ca0cd5458 100644 --- a/mobile/src/session/use-mobile-native-chat-terminal-stream.test.ts +++ b/mobile/src/session/use-mobile-native-chat-terminal-stream.test.ts @@ -1,7 +1,18 @@ import { createElement } from 'react' import { act, create, type ReactTestRenderer } from 'react-test-renderer' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { useMobileNativeChatTerminalStream } from './use-mobile-native-chat-terminal-stream' +import { + useMobileNativeChatTerminalStream, + type MobileNativeChatTerminalStreamController +} from './use-mobile-native-chat-terminal-stream' + +const emptyController = (): MobileNativeChatTerminalStreamController => ({ + notifyWebReady: () => {}, + notifyStreamReady: () => {}, + terminateStream: () => {}, + cancelRetry: () => {}, + clearRetries: () => {} +}) describe('useMobileNativeChatTerminalStream', () => { let renderer: ReactTestRenderer | null = null @@ -10,12 +21,18 @@ describe('useMobileNativeChatTerminalStream', () => { const subscribingRef = { current: new Set() } const webReadyRef = { current: new Set(['terminal-1']) } const initializedRef = { current: new Set(['terminal-1']) } - const subscribe = vi.fn((handle: string) => subscriptionsRef.current.set(handle, () => {})) + const subscribe = vi.fn((handle: string) => { + subscriptionsRef.current.set(handle, () => {}) + return true + }) const unsubscribe = vi.fn((handle: string) => subscriptionsRef.current.delete(handle)) - const notifyWebReadyRef = { current: (_handle: string, _wasAlreadyReady: boolean): void => {} } + const controllerRef = { current: emptyController() } + let previousActEnvironment: boolean | undefined beforeEach(() => { + previousActEnvironment = globalThis.IS_REACT_ACT_ENVIRONMENT globalThis.IS_REACT_ACT_ENVIRONMENT = true + vi.useFakeTimers() subscriptionsRef.current = new Map([['terminal-1', () => {}]]) subscribingRef.current = new Set() webReadyRef.current = new Set(['terminal-1']) @@ -28,11 +45,13 @@ describe('useMobileNativeChatTerminalStream', () => { afterEach(() => { act(() => renderer?.unmount()) renderer = null + vi.useRealTimers() + globalThis.IS_REACT_ACT_ENVIRONMENT = previousActEnvironment }) function Harness({ showNativeChat }: { showNativeChat: boolean }): null { harnessRenderCount += 1 - notifyWebReadyRef.current = useMobileNativeChatTerminalStream({ + controllerRef.current = useMobileNativeChatTerminalStream({ showNativeChat, activeHandle: 'terminal-1', activeTabType: 'terminal', @@ -59,7 +78,7 @@ describe('useMobileNativeChatTerminalStream', () => { renderer = create(createElement(Harness, { showNativeChat: false })) }) await act(async () => { - notifyWebReadyRef.current('terminal-1', false) + controllerRef.current.notifyWebReady('terminal-1', false) }) expect(harnessRenderCount).toBe(1) await act(async () => { @@ -103,7 +122,7 @@ describe('useMobileNativeChatTerminalStream', () => { webReadyRef.current.add('terminal-1') await act(async () => { - notifyWebReadyRef.current('terminal-1', false) + controllerRef.current.notifyWebReady('terminal-1', false) }) expect(unsubscribe).toHaveBeenNthCalledWith(2, 'terminal-1') @@ -112,4 +131,58 @@ describe('useMobileNativeChatTerminalStream', () => { consoleSpy.mockRestore() } }) + + it('retries a terminated covered lease until an acknowledgement resets backoff', async () => { + await act(async () => { + renderer = create(createElement(Harness, { showNativeChat: true })) + }) + subscribe.mockClear() + + controllerRef.current.terminateStream('terminal-1', unsubscribe) + await act(async () => vi.advanceTimersByTimeAsync(249)) + expect(subscribe).not.toHaveBeenCalled() + await act(async () => vi.advanceTimersByTimeAsync(1)) + expect(subscribe).toHaveBeenCalledWith('terminal-1') + + controllerRef.current.terminateStream('terminal-1', unsubscribe) + await act(async () => vi.advanceTimersByTimeAsync(499)) + expect(subscribe).toHaveBeenCalledOnce() + await act(async () => vi.advanceTimersByTimeAsync(1)) + expect(subscribe).toHaveBeenCalledTimes(2) + + controllerRef.current.notifyStreamReady('terminal-1') + controllerRef.current.terminateStream('terminal-1', unsubscribe) + await act(async () => vi.advanceTimersByTimeAsync(250)) + expect(subscribe).toHaveBeenCalledTimes(3) + }) + + it('cancels a covered retry when chat closes', async () => { + await act(async () => { + renderer = create(createElement(Harness, { showNativeChat: true })) + }) + subscribe.mockClear() + + controllerRef.current.terminateStream('terminal-1', unsubscribe) + await act(async () => { + renderer?.update(createElement(Harness, { showNativeChat: false })) + }) + subscribe.mockClear() + await act(async () => vi.advanceTimersByTimeAsync(4_000)) + + expect(subscribe).not.toHaveBeenCalled() + }) + + it('keeps backing off when a retry cannot arm a subscription yet', async () => { + await act(async () => { + renderer = create(createElement(Harness, { showNativeChat: true })) + }) + subscribe.mockClear().mockReturnValueOnce(false) + + controllerRef.current.terminateStream('terminal-1', unsubscribe) + await act(async () => vi.advanceTimersByTimeAsync(250)) + expect(subscribe).toHaveBeenCalledOnce() + await act(async () => vi.advanceTimersByTimeAsync(500)) + + expect(subscribe).toHaveBeenCalledTimes(2) + }) }) diff --git a/mobile/src/session/use-mobile-native-chat-terminal-stream.ts b/mobile/src/session/use-mobile-native-chat-terminal-stream.ts index c45980ae6e3..5611264bbda 100644 --- a/mobile/src/session/use-mobile-native-chat-terminal-stream.ts +++ b/mobile/src/session/use-mobile-native-chat-terminal-stream.ts @@ -1,5 +1,22 @@ -import { useCallback, useEffect, useRef, useState, type MutableRefObject } from 'react' -import { resolveMobileNativeChatTerminalStreamAction } from './mobile-native-chat-terminal-stream' +import { useCallback, useEffect, useMemo, useRef, useState, type MutableRefObject } from 'react' +import { + isTerminalCoveredByNativeChat, + mobileNativeChatTerminalRetryDelay, + resolveMobileNativeChatTerminalStreamAction +} from './mobile-native-chat-terminal-stream' + +export type MobileNativeChatTerminalStreamController = { + notifyWebReady: (handle: string, wasAlreadyReady: boolean) => void + notifyStreamReady: (handle: string) => void + terminateStream: (handle: string, unsubscribe: (handle: string) => void) => void + cancelRetry: (handle: string) => void + clearRetries: () => void +} + +type RetryEntry = { + attempt: number + timer: ReturnType | null +} /** Pauses the active terminal stream while native chat covers its mounted WebView, * then resumes from a fresh scrollback snapshot when terminal view returns. */ @@ -11,11 +28,69 @@ export function useMobileNativeChatTerminalStream(args: { subscribingRef: MutableRefObject> webReadyRef: MutableRefObject> initializedRef: MutableRefObject> - subscribe: (handle: string) => void + subscribe: (handle: string) => boolean | void unsubscribe: (handle: string) => void -}): (handle: string, wasAlreadyReady: boolean) => void { + controllerRef?: MutableRefObject +}): MobileNativeChatTerminalStreamController { const coveredHandleRef = useRef(null) const [webReadyRevision, setWebReadyRevision] = useState(0) + const retryByHandleRef = useRef(new Map()) + const subscribeRef = useRef(args.subscribe) + const stateRef = useRef({ + showNativeChat: args.showNativeChat, + activeHandle: args.activeHandle, + activeTabType: args.activeTabType + }) + subscribeRef.current = args.subscribe + stateRef.current = { + showNativeChat: args.showNativeChat, + activeHandle: args.activeHandle, + activeTabType: args.activeTabType + } + + const cancelRetry = useCallback((handle: string) => { + const retry = retryByHandleRef.current.get(handle) + if (retry?.timer) { + clearTimeout(retry.timer) + } + retryByHandleRef.current.delete(handle) + }, []) + const clearRetries = useCallback(() => { + for (const retry of retryByHandleRef.current.values()) { + if (retry.timer) { + clearTimeout(retry.timer) + } + } + retryByHandleRef.current.clear() + }, []) + const scheduleRetryRef = useRef<(handle: string) => void>(() => {}) + const scheduleRetry = useCallback( + (handle: string) => { + const current = retryByHandleRef.current.get(handle) + if (current?.timer) { + return + } + const attempt = current?.attempt ?? 0 + const timer = setTimeout(() => { + retryByHandleRef.current.set(handle, { attempt: attempt + 1, timer: null }) + const state = stateRef.current + if ( + state.activeTabType !== 'terminal' || + !isTerminalCoveredByNativeChat(state.showNativeChat, state.activeHandle, handle) + ) { + cancelRetry(handle) + return + } + if (subscribeRef.current(handle) === false) { + scheduleRetryRef.current(handle) + } + }, mobileNativeChatTerminalRetryDelay(attempt)) + retryByHandleRef.current.set(handle, { attempt, timer }) + }, + [cancelRetry] + ) + scheduleRetryRef.current = scheduleRetry + const notifyWebReady = useCallback((handle: string, wasAlreadyReady: boolean) => { // Why: ordinary WebView startups must not rerender the large session route; // only readiness that can release a native-chat lease needs reconciliation. @@ -23,11 +98,40 @@ export function useMobileNativeChatTerminalStream(args: { setWebReadyRevision((revision) => revision + 1) } }, []) + const notifyStreamReady = useCallback( + (handle: string) => { + cancelRetry(handle) + }, + [cancelRetry] + ) + const terminateStream = useCallback( + (handle: string, unsubscribe: (handle: string) => void) => { + unsubscribe(handle) + const state = stateRef.current + if ( + state.activeTabType !== 'terminal' || + !isTerminalCoveredByNativeChat(state.showNativeChat, state.activeHandle, handle) + ) { + cancelRetry(handle) + return + } + scheduleRetry(handle) + }, + [cancelRetry, scheduleRetry] + ) + + useEffect(() => clearRetries, [clearRetries]) useEffect(() => { const handle = args.activeHandle if (coveredHandleRef.current && coveredHandleRef.current !== handle) { + cancelRetry(coveredHandleRef.current) coveredHandleRef.current = null } + for (const retryHandle of retryByHandleRef.current.keys()) { + if (!isTerminalCoveredByNativeChat(args.showNativeChat, handle, retryHandle)) { + cancelRetry(retryHandle) + } + } const streamActive = handle != null && (args.subscriptionsRef.current.has(handle) || args.subscribingRef.current.has(handle)) @@ -43,6 +147,7 @@ export function useMobileNativeChatTerminalStream(args: { return } if (action === 'pause') { + cancelRetry(handle) coveredHandleRef.current = handle // Why: returning to terminal must accept the fresh scrollback snapshot; // the stream was paused while chat covered output that xterm never saw. @@ -56,6 +161,7 @@ export function useMobileNativeChatTerminalStream(args: { return } if (coveredHandleRef.current === handle) { + cancelRetry(handle) args.unsubscribe(handle) coveredHandleRef.current = null } @@ -70,7 +176,21 @@ export function useMobileNativeChatTerminalStream(args: { args.subscriptionsRef, args.unsubscribe, args.webReadyRef, + cancelRetry, webReadyRevision ]) - return notifyWebReady + const controller = useMemo( + () => ({ + notifyWebReady, + notifyStreamReady, + terminateStream, + cancelRetry, + clearRetries + }), + [cancelRetry, clearRetries, notifyStreamReady, notifyWebReady, terminateStream] + ) + if (args.controllerRef) { + args.controllerRef.current = controller + } + return controller } diff --git a/src/main/runtime/mobile-subscribe-integration.test.ts b/src/main/runtime/mobile-subscribe-integration.test.ts index ceb178ee362..0f5b527a613 100644 --- a/src/main/runtime/mobile-subscribe-integration.test.ts +++ b/src/main/runtime/mobile-subscribe-integration.test.ts @@ -272,6 +272,31 @@ describe('mobile subscribe integration', () => { expect(resizes).toEqual([]) }) + it('lease-only subscribe registers send authority without resize or query authority', async () => { + const { runtime, ptySizes, resizes } = createRuntime() + + await runtime.handleMobileLeaseSubscribe('pty-1', 'client-a') + + expect(runtime.isMobileSubscriberActive('pty-1')).toBe(true) + expect(runtime.isMobileTerminalQueryReplyAuthority('pty-1', 'client-a')).toBe(false) + expect(ptySizes.get('pty-1')).toEqual({ cols: 150, rows: 40 }) + expect(resizes).toEqual([]) + const claim = runtime.beginMobileInputFloor('pty-1', 'client-a') + expect(claim).not.toBeNull() + claim?.rollback() + }) + + it('lease-only resubscribe preserves fit hold without becoming query authority', async () => { + const { runtime } = createRuntime() + await runtime.handleMobileSubscribe('pty-1', 'client-a', { cols: 45, rows: 20 }) + expect(runtime.isMobileTerminalQueryReplyAuthority('pty-1', 'client-a')).toBe(true) + + runtime.handleMobileUnsubscribe('pty-1', 'client-a') + await runtime.handleMobileLeaseSubscribe('pty-1', 'client-a') + + expect(runtime.isMobileTerminalQueryReplyAuthority('pty-1', 'client-a')).toBe(false) + }) + it('handleMobileUnsubscribe restores PTY after debounce in auto mode', async () => { const { runtime, ptySizes } = createRuntime() await runtime.handleMobileSubscribe('pty-1', 'client-a', { cols: 45, rows: 20 }) @@ -746,6 +771,23 @@ describe('mobile subscribe integration', () => { expect(ptySizes.get('pty-1')).toEqual({ cols: 45, rows: 20 }) }) + it('late lease-only resubscribe schedules a fresh restore when it leaves', async () => { + settingsState.mobileAutoRestoreFitMs = 60_000 + const { runtime, ptySizes } = createRuntime() + await runtime.handleMobileSubscribe('pty-1', 'client-a', { cols: 45, rows: 20 }) + runtime.handleMobileUnsubscribe('pty-1', 'client-a') + + // Land after soft-leave grace so the lease rebuilds its subscriber record. + await vi.advanceTimersByTimeAsync(251) + await runtime.handleMobileLeaseSubscribe('pty-1', 'client-a') + await vi.advanceTimersByTimeAsync(60_000) + expect(ptySizes.get('pty-1')).toEqual({ cols: 45, rows: 20 }) + + runtime.handleMobileUnsubscribe('pty-1', 'client-a') + await vi.advanceTimersByTimeAsync(60_000) + expect(ptySizes.get('pty-1')).toEqual({ cols: 150, rows: 40 }) + }) + it('reclaimTerminalForDesktop with active subscriber resets mode so next subscribe re-fits', async () => { // Why: reclaim must reset mode to 'auto', else the next subscribe stays in passive watch and never re-fits to phone dims. See docs/mobile-fit-hold.md. const { runtime, ptySizes } = createRuntime() diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index 1a214ef445c..8f0b9b9c907 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -2666,6 +2666,7 @@ export class OrcaRuntimeService { { clientId: string viewport: { cols: number; rows: number } | null + leaseOnly: boolean wasResizedToPhone: boolean previousCols: number | null previousRows: number | null @@ -2740,6 +2741,7 @@ export class OrcaRuntimeService { record: { clientId: string viewport: { cols: number; rows: number } | null + leaseOnly: boolean wasResizedToPhone: boolean previousCols: number | null previousRows: number | null @@ -8601,7 +8603,7 @@ export class OrcaRuntimeService { // mutable Map order or passive desktop-mode watchers. let earliest: { clientId: string; subscribedAt: number } | null = null for (const subscriber of subscribers.values()) { - if (!subscriber.wasResizedToPhone) { + if (subscriber.leaseOnly || !subscriber.wasResizedToPhone) { continue } if (earliest === null || subscriber.subscribedAt < earliest.subscribedAt) { @@ -11318,6 +11320,7 @@ export class OrcaRuntimeService { return { updated: false, applied: false } } sub.viewport = viewport + sub.leaseOnly = false sub.lastActedAt = Date.now() const mode = this.getMobileDisplayMode(ptyId) @@ -11889,7 +11892,7 @@ export class OrcaRuntimeService { viewport?: { cols: number; rows: number } ): Promise { try { - return await this.handleMobileSubscribeInternal(ptyId, clientId, viewport) + return await this.handleMobileSubscribeInternal(ptyId, clientId, viewport, false) } finally { // Every subscribe path mutates mobileSubscribers — resync the daemon // background mark once, whatever branch returned. @@ -11897,10 +11900,19 @@ export class OrcaRuntimeService { } } + async handleMobileLeaseSubscribe(ptyId: string, clientId: string): Promise { + try { + await this.handleMobileSubscribeInternal(ptyId, clientId, undefined, true) + } finally { + this.notifyRemoteTerminalViewPresenceChanged(ptyId) + } + } + private async handleMobileSubscribeInternal( ptyId: string, clientId: string, - viewport?: { cols: number; rows: number } + viewport: { cols: number; rows: number } | undefined, + leaseOnly: boolean ): Promise { const mode = this.getMobileDisplayMode(ptyId) @@ -11927,6 +11939,7 @@ export class OrcaRuntimeService { inner.set(clientId, { ...softLeaver.record, viewport: viewport ?? null, + leaseOnly, lastActedAt: Date.now() }) if (!viewport) { @@ -11992,7 +12005,11 @@ export class OrcaRuntimeService { inner.set(clientId, { clientId, viewport: null, - wasResizedToPhone: false, + leaseOnly, + // Why: a late lease-only resubscribe owns the existing fit hold's + // eventual restore without becoming terminal-output query authority. + wasResizedToPhone: + existing?.wasResizedToPhone === true || heldOverride != null || pendingRestore != null, previousCols, previousRows, subscribedAt, @@ -12013,6 +12030,7 @@ export class OrcaRuntimeService { inner.set(clientId, { clientId, viewport, + leaseOnly, wasResizedToPhone: false, previousCols: null, previousRows: null, @@ -12025,6 +12043,7 @@ export class OrcaRuntimeService { inner.set(clientId, { clientId, viewport, + leaseOnly, wasResizedToPhone: true, previousCols, previousRows, @@ -12147,6 +12166,7 @@ export class OrcaRuntimeService { record: { clientId: subscriber.clientId, viewport: subscriber.viewport, + leaseOnly: subscriber.leaseOnly, wasResizedToPhone: subscriber.wasResizedToPhone, previousCols: subscriber.previousCols, previousRows: subscriber.previousRows, diff --git a/src/main/runtime/rpc/methods/terminal.ts b/src/main/runtime/rpc/methods/terminal.ts index 487f79f261c..9b297d1735f 100644 --- a/src/main/runtime/rpc/methods/terminal.ts +++ b/src/main/runtime/rpc/methods/terminal.ts @@ -2555,8 +2555,13 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ { runtime, connectionId, sendBinary, registerBinaryStreamHandler, signal }, emit ) => { + const readinessStartedAt = Date.now() + let ptyWaitMs = 0 let leaf = runtime.resolveLeafForHandle(params.terminal) const isMobile = params.client?.type === 'mobile' + const clientId = params.client?.id + const mobileInputLeaseOnly = + isMobile && params.capabilities?.mobileInputLeaseOnly === 1 && Boolean(clientId) const serializerGenerationBeforeAnyMount = isMobile ? (runtime.getRendererTerminalSerializerGenerationForHandle?.(params.terminal) ?? 0) : 0 @@ -2571,6 +2576,7 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ if (!leaf?.ptyId && params.client) { // Why: a never-mounted tab has no graph leaf to await; mounting the exact tab attaches its PTY without activating the worktree. rendererMountRequestedBeforePty = runtime.requestRendererTerminalTabMount(params.terminal) + const ptyWaitStartedAt = Date.now() try { const ptyId = await runtime.waitForLeafPtyId(params.terminal, 10_000, signal) leaf = { ptyId } @@ -2579,10 +2585,25 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ return } // PTY wait timed out — fall through to scrollback-only path below + } finally { + ptyWaitMs = Math.max(0, Date.now() - ptyWaitStartedAt) } } if (!leaf?.ptyId) { + if (mobileInputLeaseOnly) { + emit({ + type: 'terminal-unavailable', + reason: 'pty-not-ready', + retryable: true, + readinessTiming: { + serverTotalMs: Math.max(0, Date.now() - readinessStartedAt), + ptyWaitMs + } + }) + emit({ type: 'end' }) + return + } const read = await runtime.readTerminal(params.terminal) emit({ type: 'subscribed', @@ -2599,9 +2620,6 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ } const ptyId = leaf.ptyId - const clientId = params.client?.id - const mobileInputLeaseOnly = - isMobile && params.capabilities?.mobileInputLeaseOnly === 1 && Boolean(clientId) // Why: mount/PTY wait and phone-fit can each emit a redraw creating suffix-only state, so capture the pre-mount absence signal first. const missingHeadlessStateBeforeMobileFit = isMobile && @@ -2635,8 +2653,11 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ .then(() => runtime.cleanupSubscription(subscriptionId)) .catch(() => runtime.cleanupSubscription(subscriptionId)) try { - // Why: a lease-only subscriber has no terminal view, so its cached viewport must never phone-fit the PTY. - await runtime.handleMobileSubscribe(ptyId, clientId, undefined) + const leaseRegisterStartedAt = Date.now() + // Why: lease-only chat registers send authority without becoming a + // hidden output subscriber or awaiting terminal resize work. + await runtime.handleMobileLeaseSubscribe(ptyId, clientId) + const leaseRegisterMs = Math.max(0, Date.now() - leaseRegisterStartedAt) if (closed || signal?.aborted) { // Why: a disconnect can win the awaited subscribe and resurrect mobile presence after cleanup already released it. runtime.handleMobileUnsubscribe(ptyId, clientId) @@ -2645,7 +2666,18 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ } return } - emit({ type: 'subscribed', streamId: null, lines: [], truncated: false }) + emit({ + type: 'subscribed', + streamId: null, + lines: [], + truncated: false, + leaseReady: true, + readinessTiming: { + serverTotalMs: Math.max(0, Date.now() - readinessStartedAt), + ptyWaitMs, + leaseRegisterMs + } + }) await streamClosed } catch (error) { runtime.cleanupSubscription(subscriptionId) diff --git a/src/main/runtime/rpc/terminal-subscribe-lease-only.test.ts b/src/main/runtime/rpc/terminal-subscribe-lease-only.test.ts index 3aba4b58200..58f7391cc3a 100644 --- a/src/main/runtime/rpc/terminal-subscribe-lease-only.test.ts +++ b/src/main/runtime/rpc/terminal-subscribe-lease-only.test.ts @@ -24,7 +24,7 @@ describe('terminal lease-only subscription', () => { const runtime = { getRuntimeId: () => 'test-runtime', resolveLeafForHandle: vi.fn().mockReturnValue({ ptyId: 'pty-1' }), - handleMobileSubscribe: vi.fn().mockResolvedValue(true), + handleMobileLeaseSubscribe: vi.fn().mockResolvedValue(undefined), handleMobileUnsubscribe: vi.fn(), subscribeToTerminalData: vi.fn(), registerRemoteTerminalViewSubscriber: vi.fn(), @@ -58,7 +58,18 @@ describe('terminal lease-only subscription', () => { true ) ) - expect(runtime.handleMobileSubscribe).toHaveBeenCalledWith('pty-1', 'phone-1', undefined) + const subscribed = messages + .map((message) => JSON.parse(message).result) + .find((result) => result?.type === 'subscribed') + expect(subscribed).toMatchObject({ + leaseReady: true, + readinessTiming: { + serverTotalMs: expect.any(Number), + ptyWaitMs: 0, + leaseRegisterMs: expect.any(Number) + } + }) + expect(runtime.handleMobileLeaseSubscribe).toHaveBeenCalledWith('pty-1', 'phone-1') expect(runtime.subscribeToTerminalData).not.toHaveBeenCalled() expect(runtime.registerRemoteTerminalViewSubscriber).not.toHaveBeenCalled() expect(runtime.readTerminal).not.toHaveBeenCalled() @@ -70,4 +81,38 @@ describe('terminal lease-only subscription', () => { await dispatchPromise expect(runtime.handleMobileUnsubscribe).toHaveBeenCalledWith('pty-1', 'phone-1') }) + + it('reports retryable terminal unavailability without a false lease acknowledgement', async () => { + const messages: string[] = [] + const runtime = { + getRuntimeId: () => 'test-runtime', + resolveLeafForHandle: vi.fn().mockReturnValue(null), + requestRendererTerminalTabMount: vi.fn().mockReturnValue(true), + waitForLeafPtyId: vi.fn().mockRejectedValue(new Error('timeout')), + readTerminal: vi.fn() + } as unknown as OrcaRuntimeService + const dispatcher = new RpcDispatcher({ runtime, methods: TERMINAL_METHODS }) + + await dispatcher.dispatchStreaming(request, (message) => messages.push(message), { + connectionId: 'conn-phone', + sendBinary: vi.fn(), + registerBinaryStreamHandler: vi.fn(() => vi.fn()) + }) + + const results = messages.map((message) => JSON.parse(message).result) + expect(results).toEqual([ + expect.objectContaining({ + type: 'terminal-unavailable', + reason: 'pty-not-ready', + retryable: true, + readinessTiming: expect.objectContaining({ + serverTotalMs: expect.any(Number), + ptyWaitMs: expect.any(Number) + }) + }), + { type: 'end' } + ]) + expect(runtime.readTerminal).not.toHaveBeenCalled() + expect(results).not.toContainEqual(expect.objectContaining({ type: 'subscribed' })) + }) })