From 0586bab4f90f381f51e568c260fd45abface811e Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 3 Aug 2026 17:55:30 -0700 Subject: [PATCH] fix(mobile): bound terminal viewport resubscribe loop with backoff (STA-3337) (#12362) * fix(mobile): bound terminal viewport resubscribe loop with backoff (STA-3337) An empty scrollback frame with absent host dims was coerced to 80x24, which never equals a phone viewport, arming a zero-delay unsubscribe/resubscribe loop (~25/s) that broke long-press gestures and drained battery. - Absent host dims now hold the stream instead of resubscribing. - Fit resubscribes are budgeted per handle (3 attempts, escalating backoff) with an absence-gated refill mirroring the chat-side rearm bound; on exhaustion the view degrades visibly via toast instead of hot-looping. - A fresh post-measure match counts as convergence instead of resubscribing. - setTerminalModes keeps the Map identity when the mode is unchanged, so same-mode frames no longer re-render the session route. - Host emits the subscriber viewport as scrollback dims when the snapshot and PTY size are both unavailable, so current hosts converge immediately. * fix(mobile): cancel stale viewport retries after convergence --- .../app/h/[hostId]/session/[worktreeId].tsx | 95 ++-- .../session/mobile-terminal-diagnostics.ts | 27 +- ...bile-terminal-viewport-resubscribe.test.ts | 472 ++++++++++++++++++ .../mobile-terminal-viewport-resubscribe.ts | 293 +++++++++++ src/main/runtime/rpc/methods/terminal.ts | 12 +- 5 files changed, 850 insertions(+), 49 deletions(-) create mode 100644 mobile/src/session/mobile-terminal-viewport-resubscribe.test.ts create mode 100644 mobile/src/session/mobile-terminal-viewport-resubscribe.ts diff --git a/mobile/app/h/[hostId]/session/[worktreeId].tsx b/mobile/app/h/[hostId]/session/[worktreeId].tsx index 8f6d7ca0c..4f190c08d 100644 --- a/mobile/app/h/[hostId]/session/[worktreeId].tsx +++ b/mobile/app/h/[hostId]/session/[worktreeId].tsx @@ -235,6 +235,11 @@ import { } from '../../../../src/session/mobile-terminal-prune-decision' import { useMobileNativeChatTerminalStream } from '../../../../src/session/use-mobile-native-chat-terminal-stream' import { subscribeMobileTerminalSafely } from '../../../../src/session/mobile-terminal-stream-subscribe' +import { + TerminalViewportResubscribeBudget, + readTerminalViewportDims, + runTerminalViewportFitPass +} from '../../../../src/session/mobile-terminal-viewport-resubscribe' import { activateMobileSessionTab } from '../../../../src/session/mobile-session-tab-activation' import { MobileTerminalDiagnostics } from '../../../../src/session/mobile-terminal-diagnostics' import { runAcceptedMobileSessionTabsEffects } from '../../../../src/session/mobile-session-tabs-accepted-effects' @@ -1030,6 +1035,8 @@ export default function SessionScreen() { const subscribingHandlesRef = useRef>(new Set()) const initializedHandlesRef = useRef>(new Set()) const terminalDiagnosticsRef = useRef(new MobileTerminalDiagnostics()) + // Why: bounds the scrollback→resubscribe fit loop per handle (STA-3337). + const viewportResubscribeBudgetRef = useRef(new TerminalViewportResubscribeBudget()) // Why: don't subscribe until the WebView fires web-ready — iOS may defer JS in hidden WebViews and init() messages would queue unrendered. const webReadyHandlesRef = useRef>(new Set()) const activeHandleRef = useRef(null) @@ -1376,6 +1383,7 @@ export default function SessionScreen() { subscribingHandlesRef.current.clear() initializedHandlesRef.current.clear() terminalDiagnosticsRef.current.clearTerminalCache() + viewportResubscribeBudgetRef.current.clear() webReadyHandlesRef.current.clear() subscribeSeqRef.current.clear() layoutSeqRef.current.clear() @@ -1505,10 +1513,11 @@ export default function SessionScreen() { return } updateTerminalCwdFromStreamEvent(handle, data, terminalCwdRef.current) - const cols = (data.cols as number) || 80 - const rows = (data.rows as number) || 24 - const scrollbackCols = cols - const scrollbackRows = rows + const { hostCols, hostRows } = readTerminalViewportDims(data) + // Why: absent host dims must not be coerced into a comparable size — 80x24 + // never equals a phone viewport and armed a zero-delay resubscribe loop (STA-3337). + const cols = hostCols ?? viewportRef.current?.cols ?? 80 + const rows = hostRows ?? viewportRef.current?.rows ?? 24 const initialData = typeof data.serialized === 'string' && data.serialized.length > 0 ? data.serialized @@ -1527,46 +1536,36 @@ export default function SessionScreen() { ref.init(cols, rows, initialData, false, oscLinks) initializedHandlesRef.current.add(handle) if (data.displayMode) { + const displayMode = data.displayMode as MobileDisplayMode + // Why: same-mode frames must keep the Map identity, or every stream pass re-renders the whole route. setTerminalModes((prev) => - new Map(prev).set(handle, data.displayMode as MobileDisplayMode) + prev.get(handle) === displayMode ? prev : new Map(prev).set(handle, displayMode) ) } // Why: cold-start refit — init()'s fit can run against a transient scrollWidth, so re-fire against a settled DOM. scheduleDelayedAction(() => getTerminalRef(handle)?.resetZoom(), 200) - // Why: first subscribe has no viewport (xterm not loaded yet), so measure after init and resubscribe so the server can phone-fit. - const needsResubscribe = - !viewportMeasuredRef.current || - (viewportRef.current != null && - (scrollbackCols !== viewportRef.current.cols || - scrollbackRows !== viewportRef.current.rows)) - if (needsResubscribe) { - void (async () => { - // Why: wait for init()'s rAF chain before measuring, else the measure races ahead and returns null (log dump 2026-05-06). - await getTerminalRef(handle)?.awaitReady() - if (subscribeSeqRef.current.get(handle) !== seq) { - return - } - const dims = await getTerminalRef(handle)?.measureFitDimensions( - terminalFrameHeightRef.current || undefined - ) - // Why: re-check seq — the awaits may have let a newer subscribe cycle arm; tearing it down would resubscribe a stale generation. - if (subscribeSeqRef.current.get(handle) !== seq) { - return - } - if (!getTerminalRef(handle)) { - return - } - // Why: scrollback came back at cols=80 (server's null-viewport fallback), so this subscriber record has no viewport — resubscribe so the server stores it. - if (dims) { - diagnostics.streamResubscribing(handle, seq, dims) - viewportRef.current = dims - viewportMeasuredRef.current = true - unsubscribeTerminal(handle) - initializedHandlesRef.current.delete(handle) - subscribeToTerminal(handle) - } - })() - } + // Why: first subscribe has no viewport (xterm not loaded yet), so measure after init + // and resubscribe so the server can phone-fit — bounded per handle so a + // non-converging host degrades visibly instead of hot-looping (STA-3337). + runTerminalViewportFitPass({ + handle, + seq, + hostCols, + hostRows, + budget: viewportResubscribeBudgetRef.current, + diagnostics, + viewportRef, + viewportMeasuredRef, + subscribeSeqRef, + initializedHandlesRef, + terminalUnsubsRef, + terminalFrameHeightRef, + getTerminalRef, + unsubscribeTerminal, + subscribeToTerminal, + scheduleDelayedAction, + showToast + }) } else if (data.type === 'metadata') { updateTerminalCwdFromStreamEvent(handle, data, terminalCwdRef.current) } else if (data.type === 'data') { @@ -1591,8 +1590,12 @@ export default function SessionScreen() { } else if (data.type === 'resized') { updateTerminalCwdFromStreamEvent(handle, data, terminalCwdRef.current) // Server resize: reinit xterm on a full-buffer snapshot (width reflow rewraps scrollback), else just resize geometry. - const cols = (data.cols as number) || 80 - const rows = (data.rows as number) || 24 + const viewport = viewportMeasuredRef.current ? viewportRef.current : null + const [cols, rows] = viewportResubscribeBudgetRef.current.observeResize( + handle, + data, + viewport + ) const serialized = typeof data.serialized === 'string' ? data.serialized : null diagnostics.streamResized(handle, seq, eventSeq, data, getTerminalRef(handle) != null) const oscLinks = isTerminalOscLinkRanges(data.oscLinks) ? data.oscLinks : undefined @@ -1602,8 +1605,10 @@ export default function SessionScreen() { getTerminalRef(handle)?.resize(cols, rows) } if (data.displayMode) { + const displayMode = data.displayMode as MobileDisplayMode + // Why: same-mode frames must keep the Map identity, or every stream pass re-renders the whole route. setTerminalModes((prev) => - new Map(prev).set(handle, data.displayMode as MobileDisplayMode) + prev.get(handle) === displayMode ? prev : new Map(prev).set(handle, displayMode) ) } scheduleDelayedAction(() => getTerminalRef(handle)?.resetZoom(), 200) @@ -1619,7 +1624,7 @@ export default function SessionScreen() { } subscribingHandlesRef.current.delete(handle) }, - [client, getTerminalRef, markNativeChatInputLeaseReady, scheduleDelayedAction] + [client, getTerminalRef, markNativeChatInputLeaseReady, scheduleDelayedAction, showToast] ) const nativeChatStream = useMobileNativeChatTerminalStream({ @@ -1720,12 +1725,16 @@ export default function SessionScreen() { unsubscribeTerminal(handle) terminalRefs.current.delete(handle) initializedHandlesRef.current.delete(handle) + viewportResubscribeBudgetRef.current.forget(handle) clearTerminalLiveInputDefault(handle) } setTerminalKeyboardMetrics((prev) => pruneTerminalKeyboardMetrics(prev, shouldPrune)) // Why: a chat-covered handle the host reports again refills its rearm budget, // so an exhausted rearm can't lock the composer until leave-chat. nativeChatStream.notifyListedHandles(liveHandles) + // Why: same absence-gated refill for the viewport-fit budget — a handle that + // left the list and returned may converge now, so it earns fresh attempts. + viewportResubscribeBudgetRef.current.notifyListedHandles(liveHandles) lastKnownTerminalCountRef.current = result.terminals.length // Why: dedupe duplicate handles (rename/split race) to avoid a React duplicate-key throw; keep first for tab-strip order. const seen = new Set() diff --git a/mobile/src/session/mobile-terminal-diagnostics.ts b/mobile/src/session/mobile-terminal-diagnostics.ts index 8695928ed..6ee636fb9 100644 --- a/mobile/src/session/mobile-terminal-diagnostics.ts +++ b/mobile/src/session/mobile-terminal-diagnostics.ts @@ -141,12 +141,35 @@ export class MobileTerminalDiagnostics { }) } - streamResubscribing(handle: string, seq: number, dims: { cols: number; rows: number }): void { + streamResubscribing( + handle: string, + seq: number, + dims: { cols: number; rows: number }, + attempt: number, + delayMs: number + ): void { logMobileTerminalDiagnostic('stream-resubscribe-for-viewport', { handle: shortenMobileTerminalDiagnosticId(handle), seq, cols: dims.cols, - rows: dims.rows + rows: dims.rows, + attempt, + delayMs + }) + } + + streamResubscribeHeld(handle: string, seq: number): void { + logMobileTerminalDiagnostic('stream-resubscribe-held-absent-dims', { + handle: shortenMobileTerminalDiagnosticId(handle), + seq + }) + } + + streamResubscribeExhausted(handle: string, seq: number, attempts: number): void { + logMobileTerminalDiagnostic('stream-resubscribe-exhausted', { + handle: shortenMobileTerminalDiagnosticId(handle), + seq, + attempts }) } diff --git a/mobile/src/session/mobile-terminal-viewport-resubscribe.test.ts b/mobile/src/session/mobile-terminal-viewport-resubscribe.test.ts new file mode 100644 index 000000000..35aac89f6 --- /dev/null +++ b/mobile/src/session/mobile-terminal-viewport-resubscribe.test.ts @@ -0,0 +1,472 @@ +import { describe, expect, it, vi } from 'vitest' +import { + MAX_TERMINAL_VIEWPORT_RESUBSCRIBE_ATTEMPTS, + TerminalViewportResubscribeBudget, + readTerminalViewportDims, + resolveTerminalViewportResubscribe, + runTerminalViewportFitPass, + shouldResubscribeAfterViewportMeasure, + type TerminalViewportFitPassArgs +} from './mobile-terminal-viewport-resubscribe' + +const PHONE = { cols: 40, rows: 50 } + +describe('readTerminalViewportDims', () => { + it('accepts only usable numeric host dimensions', () => { + expect(readTerminalViewportDims({ cols: 40, rows: 50 })).toEqual({ + hostCols: 40, + hostRows: 50 + }) + expect(readTerminalViewportDims({ cols: Number.NaN, rows: 0 })).toEqual({ + hostCols: null, + hostRows: null + }) + }) +}) + +describe('resolveTerminalViewportResubscribe', () => { + it('resubscribes immediately on the first pass when the viewport is unmeasured', () => { + expect( + resolveTerminalViewportResubscribe({ + hostCols: 80, + hostRows: 24, + viewportMeasured: false, + viewport: null, + attempts: 0 + }) + ).toEqual({ kind: 'resubscribe', delayMs: 0 }) + }) + + it('caps even the unmeasured pass once the budget is spent', () => { + expect( + resolveTerminalViewportResubscribe({ + hostCols: 80, + hostRows: 24, + viewportMeasured: false, + viewport: null, + attempts: MAX_TERMINAL_VIEWPORT_RESUBSCRIBE_ATTEMPTS + }) + ).toEqual({ kind: 'exhausted' }) + }) + + it('holds on absent host dims instead of resubscribing (STA-3337 regression)', () => { + for (const [hostCols, hostRows] of [ + [null, null], + [80, null], + [null, 24] + ] as const) { + expect( + resolveTerminalViewportResubscribe({ + hostCols, + hostRows, + viewportMeasured: true, + viewport: PHONE, + attempts: 0 + }) + ).toEqual({ kind: 'hold' }) + } + }) + + it('keeps holding on absent dims across repeated frames without spending budget', () => { + for (let frame = 0; frame < 50; frame += 1) { + expect( + resolveTerminalViewportResubscribe({ + hostCols: null, + hostRows: null, + viewportMeasured: true, + viewport: PHONE, + attempts: 0 + }).kind + ).toBe('hold') + } + }) + + it('converges when host dims match the measured viewport', () => { + expect( + resolveTerminalViewportResubscribe({ + hostCols: PHONE.cols, + hostRows: PHONE.rows, + viewportMeasured: true, + viewport: PHONE, + attempts: 2 + }) + ).toEqual({ kind: 'converged' }) + }) + + it('backs off across mismatch attempts and then exhausts', () => { + const delays = [0, 1, 2].map((attempts) => { + const decision = resolveTerminalViewportResubscribe({ + hostCols: 80, + hostRows: 24, + viewportMeasured: true, + viewport: PHONE, + attempts + }) + if (decision.kind !== 'resubscribe') { + throw new Error(`expected resubscribe at attempt ${attempts}, got ${decision.kind}`) + } + return decision.delayMs + }) + expect(delays[0]).toBe(0) + expect(delays[1]).toBeGreaterThan(0) + expect(delays[2]).toBeGreaterThan(delays[1]) + expect( + resolveTerminalViewportResubscribe({ + hostCols: 80, + hostRows: 24, + viewportMeasured: true, + viewport: PHONE, + attempts: MAX_TERMINAL_VIEWPORT_RESUBSCRIBE_ATTEMPTS + }) + ).toEqual({ kind: 'exhausted' }) + }) +}) + +describe('shouldResubscribeAfterViewportMeasure', () => { + it('always resubscribes when the viewport was never measured (server must learn it)', () => { + expect( + shouldResubscribeAfterViewportMeasure({ + hostCols: PHONE.cols, + hostRows: PHONE.rows, + measured: PHONE, + viewportWasMeasured: false + }) + ).toBe(true) + }) + + it('skips the resubscribe when the fresh measure already matches the host', () => { + expect( + shouldResubscribeAfterViewportMeasure({ + hostCols: PHONE.cols, + hostRows: PHONE.rows, + measured: PHONE, + viewportWasMeasured: true + }) + ).toBe(false) + }) + + it('resubscribes when the host still disagrees with the fresh measure', () => { + expect( + shouldResubscribeAfterViewportMeasure({ + hostCols: 80, + hostRows: 24, + measured: PHONE, + viewportWasMeasured: true + }) + ).toBe(true) + }) +}) + +describe('TerminalViewportResubscribeBudget', () => { + const exhaust = (budget: TerminalViewportResubscribeBudget, handle: string) => { + for (let i = 0; i < MAX_TERMINAL_VIEWPORT_RESUBSCRIBE_ATTEMPTS; i += 1) { + budget.chargeAttempt(handle) + } + } + + it('starts at zero and counts charged attempts per handle', () => { + const budget = new TerminalViewportResubscribeBudget() + expect(budget.attempts('t1')).toBe(0) + budget.chargeAttempt('t1') + budget.chargeAttempt('t1') + expect(budget.attempts('t1')).toBe(2) + expect(budget.attempts('t2')).toBe(0) + }) + + it('resets attempts and re-arms the announcement on convergence', () => { + const budget = new TerminalViewportResubscribeBudget() + exhaust(budget, 't1') + expect(budget.shouldAnnounceExhaustion('t1')).toBe(true) + budget.markConverged('t1') + expect(budget.attempts('t1')).toBe(0) + expect(budget.shouldAnnounceExhaustion('t1')).toBe(true) + }) + + it('announces exhaustion exactly once', () => { + const budget = new TerminalViewportResubscribeBudget() + exhaust(budget, 't1') + expect(budget.shouldAnnounceExhaustion('t1')).toBe(true) + expect(budget.shouldAnnounceExhaustion('t1')).toBe(false) + }) + + it('does not refill an exhausted handle that stayed listed', () => { + const budget = new TerminalViewportResubscribeBudget() + exhaust(budget, 't1') + for (let refresh = 0; refresh < 5; refresh += 1) { + budget.notifyListedHandles(new Set(['t1'])) + } + expect(budget.attempts('t1')).toBe(MAX_TERMINAL_VIEWPORT_RESUBSCRIBE_ATTEMPTS) + }) + + it('refills only after the exhausted handle went absent and came back', () => { + const budget = new TerminalViewportResubscribeBudget() + exhaust(budget, 't1') + expect(budget.shouldAnnounceExhaustion('t1')).toBe(true) + budget.notifyListedHandles(new Set()) + // Still exhausted while absent — the refill lands on the return. + expect(budget.attempts('t1')).toBe(MAX_TERMINAL_VIEWPORT_RESUBSCRIBE_ATTEMPTS) + budget.notifyListedHandles(new Set(['t1'])) + expect(budget.attempts('t1')).toBe(0) + expect(budget.shouldAnnounceExhaustion('t1')).toBe(true) + }) + + it('leaves below-cap handles untouched by list refreshes', () => { + const budget = new TerminalViewportResubscribeBudget() + budget.chargeAttempt('t1') + budget.notifyListedHandles(new Set()) + budget.notifyListedHandles(new Set(['t1'])) + expect(budget.attempts('t1')).toBe(1) + }) + + it('forget clears one handle, clear clears everything', () => { + const budget = new TerminalViewportResubscribeBudget() + exhaust(budget, 't1') + exhaust(budget, 't2') + budget.forget('t1') + expect(budget.attempts('t1')).toBe(0) + expect(budget.attempts('t2')).toBe(MAX_TERMINAL_VIEWPORT_RESUBSCRIBE_ATTEMPTS) + budget.clear() + expect(budget.attempts('t2')).toBe(0) + }) +}) + +describe('runTerminalViewportFitPass', () => { + const HANDLE = 't1' + + function makeHarness(overrides: { + hostCols?: number | null + hostRows?: number | null + viewportMeasured?: boolean + viewport?: { cols: number; rows: number } | null + measured?: { cols: number; rows: number } | null + budget?: TerminalViewportResubscribeBudget + }) { + const budget = overrides.budget ?? new TerminalViewportResubscribeBudget() + const diagnostics = { + streamResubscribing: vi.fn(), + streamResubscribeHeld: vi.fn(), + streamResubscribeExhausted: vi.fn() + } + const terminalUnsubsRef = { current: new Map void>([[HANDLE, () => {}]]) } + const scheduled: { fn: () => void; ms: number }[] = [] + const webView = { + awaitReady: () => Promise.resolve(), + measureFitDimensions: () => Promise.resolve(overrides.measured ?? PHONE) + } + const unsubscribeTerminal = vi.fn((handle: string) => { + terminalUnsubsRef.current.delete(handle) + }) + // Mimic the real subscribe path: arming registers an unsubscribe handle. + const subscribeToTerminal = vi.fn((handle: string) => { + terminalUnsubsRef.current.set(handle, () => {}) + }) + const showToast = vi.fn() + const args: TerminalViewportFitPassArgs = { + handle: HANDLE, + seq: 1, + hostCols: overrides.hostCols ?? null, + hostRows: overrides.hostRows ?? null, + budget, + diagnostics, + viewportRef: { current: overrides.viewport ?? null }, + viewportMeasuredRef: { current: overrides.viewportMeasured ?? false }, + subscribeSeqRef: { current: new Map([[HANDLE, 1]]) }, + initializedHandlesRef: { current: new Set([HANDLE]) }, + terminalUnsubsRef, + terminalFrameHeightRef: { current: 0 }, + getTerminalRef: () => webView, + unsubscribeTerminal, + subscribeToTerminal, + scheduleDelayedAction: (fn, ms) => scheduled.push({ fn, ms }), + showToast + } + return { + args, + budget, + diagnostics, + scheduled, + subscribeToTerminal, + unsubscribeTerminal, + showToast + } + } + + const settle = () => new Promise((resolve) => setTimeout(resolve, 0)) + + it('holds without any teardown when host dims are absent (STA-3337 regression)', async () => { + const h = makeHarness({ viewportMeasured: true, viewport: PHONE }) + runTerminalViewportFitPass(h.args) + await settle() + expect(h.unsubscribeTerminal).not.toHaveBeenCalled() + expect(h.subscribeToTerminal).not.toHaveBeenCalled() + expect(h.diagnostics.streamResubscribeHeld).toHaveBeenCalledTimes(1) + }) + + it('measures and resubscribes immediately on the first pass, charging one attempt', async () => { + const h = makeHarness({ hostCols: 80, hostRows: 24 }) + runTerminalViewportFitPass(h.args) + await settle() + expect(h.unsubscribeTerminal).toHaveBeenCalledTimes(1) + expect(h.subscribeToTerminal).toHaveBeenCalledTimes(1) + expect(h.args.viewportRef.current).toEqual(PHONE) + expect(h.args.viewportMeasuredRef.current).toBe(true) + expect(h.budget.attempts(HANDLE)).toBe(1) + }) + + it('treats an equal fresh measure as convergence instead of resubscribing', async () => { + const budget = new TerminalViewportResubscribeBudget() + budget.chargeAttempt(HANDLE) + // Stale cached viewport disagrees with the host, but the fresh measure matches it. + const h = makeHarness({ + hostCols: PHONE.cols, + hostRows: PHONE.rows, + viewportMeasured: true, + viewport: { cols: 30, rows: 20 }, + measured: PHONE, + budget + }) + runTerminalViewportFitPass(h.args) + await settle() + expect(h.subscribeToTerminal).not.toHaveBeenCalled() + expect(h.budget.attempts(HANDLE)).toBe(0) + }) + + it('defers later attempts through the backoff scheduler while keeping the stream up', async () => { + const budget = new TerminalViewportResubscribeBudget() + budget.chargeAttempt(HANDLE) + const h = makeHarness({ + hostCols: 80, + hostRows: 24, + viewportMeasured: true, + viewport: PHONE, + budget + }) + runTerminalViewportFitPass(h.args) + await settle() + expect(h.scheduled).toHaveLength(1) + expect(h.scheduled[0].ms).toBeGreaterThan(0) + // The stream must still be up until the deferred retry fires. + expect(h.unsubscribeTerminal).not.toHaveBeenCalled() + h.scheduled[0].fn() + expect(h.unsubscribeTerminal).toHaveBeenCalledTimes(1) + expect(h.subscribeToTerminal).toHaveBeenCalledTimes(1) + expect(h.budget.attempts(HANDLE)).toBe(2) + }) + + it('drops a deferred retry whose subscribe generation went stale', async () => { + const budget = new TerminalViewportResubscribeBudget() + budget.chargeAttempt(HANDLE) + const h = makeHarness({ + hostCols: 80, + hostRows: 24, + viewportMeasured: true, + viewport: PHONE, + budget + }) + runTerminalViewportFitPass(h.args) + await settle() + expect(h.scheduled).toHaveLength(1) + h.args.subscribeSeqRef.current.set(HANDLE, 2) + h.scheduled[0].fn() + expect(h.unsubscribeTerminal).not.toHaveBeenCalled() + expect(h.budget.attempts(HANDLE)).toBe(1) + }) + + it('drops a deferred retry after the live stream converges', async () => { + const budget = new TerminalViewportResubscribeBudget() + budget.chargeAttempt(HANDLE) + const h = makeHarness({ + hostCols: 80, + hostRows: 24, + viewportMeasured: true, + viewport: PHONE, + budget + }) + runTerminalViewportFitPass(h.args) + await settle() + expect(h.scheduled).toHaveLength(1) + expect(h.budget.observeResize(HANDLE, PHONE, PHONE)).toEqual([PHONE.cols, PHONE.rows]) + h.scheduled[0].fn() + expect(h.unsubscribeTerminal).not.toHaveBeenCalled() + expect(h.subscribeToTerminal).not.toHaveBeenCalled() + expect(h.budget.attempts(HANDLE)).toBe(0) + }) + + it('announces exhaustion once and stops touching the stream', async () => { + const budget = new TerminalViewportResubscribeBudget() + for (let i = 0; i < MAX_TERMINAL_VIEWPORT_RESUBSCRIBE_ATTEMPTS; i += 1) { + budget.chargeAttempt(HANDLE) + } + const h = makeHarness({ + hostCols: 80, + hostRows: 24, + viewportMeasured: true, + viewport: PHONE, + budget + }) + runTerminalViewportFitPass(h.args) + runTerminalViewportFitPass(h.args) + await settle() + expect(h.unsubscribeTerminal).not.toHaveBeenCalled() + expect(h.subscribeToTerminal).not.toHaveBeenCalled() + expect(h.showToast).toHaveBeenCalledTimes(1) + expect(h.diagnostics.streamResubscribeExhausted).toHaveBeenCalledTimes(2) + }) +}) + +describe('STA-3337 stream shapes', () => { + it('empty scrollback with absent dims settles after a single register pass', () => { + const budget = new TerminalViewportResubscribeBudget() + // Pass 1: no viewport yet — measure and resubscribe so the server learns it. + const first = resolveTerminalViewportResubscribe({ + hostCols: null, + hostRows: null, + viewportMeasured: false, + viewport: null, + attempts: budget.attempts('t1') + }) + expect(first).toEqual({ kind: 'resubscribe', delayMs: 0 }) + budget.chargeAttempt('t1') + // Pass 2+: host still reports no dims — the stream must be left alone. + for (let frame = 0; frame < 10; frame += 1) { + expect( + resolveTerminalViewportResubscribe({ + hostCols: null, + hostRows: null, + viewportMeasured: true, + viewport: PHONE, + attempts: budget.attempts('t1') + }).kind + ).toBe('hold') + } + expect(budget.attempts('t1')).toBe(1) + }) + + it('non-converging numeric dims degrade after the bounded backoff run', () => { + const budget = new TerminalViewportResubscribeBudget() + const kinds: string[] = [] + for (let frame = 0; frame < 6; frame += 1) { + const decision = resolveTerminalViewportResubscribe({ + hostCols: 80, + hostRows: 24, + viewportMeasured: frame > 0, + viewport: frame > 0 ? PHONE : null, + attempts: budget.attempts('t1') + }) + kinds.push(decision.kind) + if (decision.kind === 'resubscribe') { + budget.chargeAttempt('t1') + } + } + expect(kinds).toEqual([ + 'resubscribe', + 'resubscribe', + 'resubscribe', + 'exhausted', + 'exhausted', + 'exhausted' + ]) + expect(budget.shouldAnnounceExhaustion('t1')).toBe(true) + expect(budget.shouldAnnounceExhaustion('t1')).toBe(false) + }) +}) diff --git a/mobile/src/session/mobile-terminal-viewport-resubscribe.ts b/mobile/src/session/mobile-terminal-viewport-resubscribe.ts new file mode 100644 index 000000000..8ad3c99c1 --- /dev/null +++ b/mobile/src/session/mobile-terminal-viewport-resubscribe.ts @@ -0,0 +1,293 @@ +/** Bounds the scrollback→measure→resubscribe fit loop (STA-3337): a host whose + * frame dims can never equal the phone viewport must not re-arm the stream + * forever — it broke gesture recognition and drained battery at ~25 cycles/s. */ + +import type { MobileTerminalDiagnostics } from './mobile-terminal-diagnostics' + +export const MAX_TERMINAL_VIEWPORT_RESUBSCRIBE_ATTEMPTS = 3 + +/** Attempt-indexed teardown delay. Attempt 0 is the ordinary first fit pass + * (server learns the viewport) and must stay immediate; later attempts mean + * the server answered with non-matching dims, so probe at a decaying rate. */ +const TERMINAL_VIEWPORT_RESUBSCRIBE_BACKOFF_MS = [0, 750, 3000] as const + +export type TerminalViewportDims = { readonly cols: number; readonly rows: number } + +function readPositiveDimension(value: unknown): number | null { + return typeof value === 'number' && Number.isFinite(value) && value > 0 ? value : null +} + +export function readTerminalViewportDims(data: Readonly>): { + readonly hostCols: number | null + readonly hostRows: number | null +} { + return { + hostCols: readPositiveDimension(data.cols), + hostRows: readPositiveDimension(data.rows) + } +} + +export type TerminalViewportResubscribeDecision = + | { readonly kind: 'resubscribe'; readonly delayMs: number } + | { readonly kind: 'converged' } + | { readonly kind: 'hold' } + | { readonly kind: 'exhausted' } + +function resubscribeDelayMs(attempts: number): number { + return TERMINAL_VIEWPORT_RESUBSCRIBE_BACKOFF_MS[ + Math.min(attempts, TERMINAL_VIEWPORT_RESUBSCRIBE_BACKOFF_MS.length - 1) + ] +} + +export function resolveTerminalViewportResubscribe(args: { + hostCols: number | null + hostRows: number | null + viewportMeasured: boolean + viewport: TerminalViewportDims | null + attempts: number +}): TerminalViewportResubscribeDecision { + const overBudget = args.attempts >= MAX_TERMINAL_VIEWPORT_RESUBSCRIBE_ATTEMPTS + // First subscribe carries no viewport; resubscribing is how the server learns it. + if (!args.viewportMeasured || args.viewport == null) { + return overBudget ? { kind: 'exhausted' } : { kind: 'resubscribe', delayMs: 0 } + } + // Why: a host that doesn't report PTY dims can never converge — resubscribing + // replays the identical frame, so keep the stream instead of probing it. + if (args.hostCols == null || args.hostRows == null) { + return { kind: 'hold' } + } + if (args.hostCols === args.viewport.cols && args.hostRows === args.viewport.rows) { + return { kind: 'converged' } + } + return overBudget + ? { kind: 'exhausted' } + : { kind: 'resubscribe', delayMs: resubscribeDelayMs(args.attempts) } +} + +/** Post-measure re-check: the pre-measure mismatch may have been a stale cached + * viewport. Resubscribing is only productive when the server still disagrees + * with the fresh measure, or was never told the viewport at all. */ +export function shouldResubscribeAfterViewportMeasure(args: { + hostCols: number | null + hostRows: number | null + measured: TerminalViewportDims + viewportWasMeasured: boolean +}): boolean { + if (!args.viewportWasMeasured) { + return true + } + return args.hostCols !== args.measured.cols || args.hostRows !== args.measured.rows +} + +/** Per-handle resubscribe budget, mirroring the chat-side rearm bound: attempts + * refill only when the handle actually left terminal.list and came back. A + * still-listed non-converging handle re-funded on every list refresh would undo + * the bound this class exists to enforce. */ +export class TerminalViewportResubscribeBudget { + private readonly attemptsByHandle = new Map() + private readonly absentSinceExhaustion = new Set() + private readonly announcedExhaustion = new Set() + private readonly retryGenerationByHandle = new Map() + + attempts(handle: string): number { + return this.attemptsByHandle.get(handle) ?? 0 + } + + chargeAttempt(handle: string): void { + this.attemptsByHandle.set(handle, this.attempts(handle) + 1) + } + + retryGeneration(handle: string): object { + const existing = this.retryGenerationByHandle.get(handle) + if (existing) { + return existing + } + const generation = {} + this.retryGenerationByHandle.set(handle, generation) + return generation + } + + isRetryGenerationCurrent(handle: string, generation: object): boolean { + return this.retryGenerationByHandle.get(handle) === generation + } + + observeResize( + handle: string, + data: Readonly>, + viewport: TerminalViewportDims | null + ): readonly [number, number] { + const { hostCols, hostRows } = readTerminalViewportDims(data) + const cols = hostCols ?? 80 + const rows = hostRows ?? 24 + if (viewport?.cols === cols && viewport.rows === rows) { + this.markConverged(handle) + } + return [cols, rows] + } + + markConverged(handle: string): void { + this.forget(handle) + } + + /** True exactly once per exhaustion so the degraded state is announced, not spammed. */ + shouldAnnounceExhaustion(handle: string): boolean { + if (this.announcedExhaustion.has(handle)) { + return false + } + this.announcedExhaustion.add(handle) + return true + } + + notifyListedHandles(liveHandles: ReadonlySet): void { + for (const handle of Array.from(this.attemptsByHandle.keys())) { + if (this.attempts(handle) < MAX_TERMINAL_VIEWPORT_RESUBSCRIBE_ATTEMPTS) { + continue + } + if (!liveHandles.has(handle)) { + this.absentSinceExhaustion.add(handle) + continue + } + // Why: only an absence marker buys a refill — the handle's PTY may be live + // again, so a fresh budget (and a fresh degrade announcement) is warranted. + if (this.absentSinceExhaustion.delete(handle)) { + this.forget(handle) + } + } + } + + forget(handle: string): void { + this.attemptsByHandle.delete(handle) + this.absentSinceExhaustion.delete(handle) + this.announcedExhaustion.delete(handle) + this.retryGenerationByHandle.delete(handle) + } + + clear(): void { + this.attemptsByHandle.clear() + this.absentSinceExhaustion.clear() + this.announcedExhaustion.clear() + this.retryGenerationByHandle.clear() + } +} + +type MutableRef = { current: T } + +type TerminalFitWebView = { + awaitReady: () => Promise + measureFitDimensions: (frameHeight?: number) => Promise +} + +export type TerminalViewportFitPassArgs = { + handle: string + seq: number + hostCols: number | null + hostRows: number | null + budget: TerminalViewportResubscribeBudget + diagnostics: Pick< + MobileTerminalDiagnostics, + 'streamResubscribing' | 'streamResubscribeHeld' | 'streamResubscribeExhausted' + > + viewportRef: MutableRef + viewportMeasuredRef: MutableRef + subscribeSeqRef: MutableRef> + initializedHandlesRef: MutableRef> + terminalUnsubsRef: MutableRef void>> + terminalFrameHeightRef: MutableRef + getTerminalRef: (handle: string | null) => TerminalFitWebView | undefined + unsubscribeTerminal: (handle: string) => void + subscribeToTerminal: (handle: string) => void + scheduleDelayedAction: (fn: () => void, ms: number) => void + showToast: (message: string, durationMs?: number) => void +} + +/** One bounded fit pass per scrollback frame: converge, hold, degrade visibly, + * or measure and resubscribe (backing off) so the server can phone-fit. */ +export function runTerminalViewportFitPass(args: TerminalViewportFitPassArgs): void { + const { handle, seq, hostCols, hostRows, budget, diagnostics } = args + const retryGeneration = budget.retryGeneration(handle) + const decision = resolveTerminalViewportResubscribe({ + hostCols, + hostRows, + viewportMeasured: args.viewportMeasuredRef.current, + viewport: args.viewportRef.current, + attempts: budget.attempts(handle) + }) + if (decision.kind === 'converged') { + budget.markConverged(handle) + return + } + if (decision.kind === 'hold') { + diagnostics.streamResubscribeHeld(handle, seq) + return + } + if (decision.kind === 'exhausted') { + diagnostics.streamResubscribeExhausted(handle, seq, budget.attempts(handle)) + if (budget.shouldAnnounceExhaustion(handle)) { + args.showToast("Couldn't fit the terminal to this screen", 4000) + } + return + } + const viewportWasMeasured = args.viewportMeasuredRef.current + void (async () => { + // Why: wait for init()'s rAF chain before measuring, else the measure races ahead and returns null (log dump 2026-05-06). + await args.getTerminalRef(handle)?.awaitReady() + if ( + args.subscribeSeqRef.current.get(handle) !== seq || + !budget.isRetryGenerationCurrent(handle, retryGeneration) + ) { + return + } + const dims = await args + .getTerminalRef(handle) + ?.measureFitDimensions(args.terminalFrameHeightRef.current || undefined) + // Why: re-check seq — the awaits may have let a newer subscribe cycle arm; tearing it down would resubscribe a stale generation. + if ( + args.subscribeSeqRef.current.get(handle) !== seq || + !budget.isRetryGenerationCurrent(handle, retryGeneration) + ) { + return + } + if (!args.getTerminalRef(handle) || !dims) { + return + } + args.viewportRef.current = dims + args.viewportMeasuredRef.current = true + if ( + !shouldResubscribeAfterViewportMeasure({ + hostCols, + hostRows, + measured: dims, + viewportWasMeasured + }) + ) { + // Why: the pre-measure mismatch was a stale cached viewport; the server already agrees. + budget.markConverged(handle) + return + } + const resubscribe = (): void => { + if ( + args.subscribeSeqRef.current.get(handle) !== seq || + !budget.isRetryGenerationCurrent(handle, retryGeneration) + ) { + return + } + if (!args.getTerminalRef(handle)) { + return + } + diagnostics.streamResubscribing(handle, seq, dims, budget.attempts(handle), decision.delayMs) + args.unsubscribeTerminal(handle) + args.initializedHandlesRef.current.delete(handle) + args.subscribeToTerminal(handle) + // Why: only a resubscribe that actually armed spends budget; one turned away by its own gates never reached the host. + if (args.terminalUnsubsRef.current.has(handle)) { + budget.chargeAttempt(handle) + } + } + if (decision.delayMs > 0) { + // Why: keep the live stream up through the backoff so input keeps flowing; teardown happens only when the retry fires. + args.scheduleDelayedAction(resubscribe, decision.delayMs) + } else { + resubscribe() + } + })() +} diff --git a/src/main/runtime/rpc/methods/terminal.ts b/src/main/runtime/rpc/methods/terminal.ts index 450437f58..d20524749 100644 --- a/src/main/runtime/rpc/methods/terminal.ts +++ b/src/main/runtime/rpc/methods/terminal.ts @@ -2974,8 +2974,10 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ serialized: serialized?.data, oscLinks: serialized?.oscLinks, cwd: serialized?.cwd, - cols: serialized?.cols ?? size?.cols, - rows: serialized?.rows ?? size?.rows, + // Why: an empty snapshot with no PTY size must still report the dims the fit + // will produce — dimless frames re-armed the mobile fit loop (STA-3337). + cols: serialized?.cols ?? size?.cols ?? params.viewport?.cols, + rows: serialized?.rows ?? size?.rows ?? params.viewport?.rows, displayMode, seq }) @@ -3381,8 +3383,10 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ }) const snapshotStats = sendSnapshotFrames(sendFrame, { kind: 'scrollback', - cols: serialized?.cols ?? size?.cols ?? 80, - rows: serialized?.rows ?? size?.rows ?? 24, + // Why: prefer the subscriber's viewport over the 80x24 stopgap when the PTY has + // no size yet — the mismatch made mobile burn its resubscribe budget (STA-3337). + cols: serialized?.cols ?? size?.cols ?? params.viewport?.cols ?? 80, + rows: serialized?.rows ?? size?.rows ?? params.viewport?.rows ?? 24, displayMode, seq: snapshotFrameSeq, cwd: serialized?.cwd,