From e04ca97dc227cb7c649989e36a2745d8ef975532 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 14 Jul 2026 01:33:27 -0700 Subject: [PATCH] fix(mobile): re-fit terminal PTY when the frame height settles (#8647) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(mobile): re-fit terminal PTY when the frame height settles A freshly-created agent terminal fits its PTY to rows = floor(frameHeight / cellHeight) before the accessory/live-input dock has laid out, so the frame is briefly too tall and the PTY gets too many rows. Claude/Codex pin their input box to the bottom of the grid, so those extra bottom rows — the input box and status lines — render behind the dock and you can't see what you're typing. Leaving and re-entering the workspace worked around it by re-measuring against the settled layout. The refit hook previously re-fit only on width changes and deliberately ignored height-only changes, so the over-fit was never corrected. Track the measured frame height and re-fit on its change too, mirroring the width path. Safe because Expo SDK 55's edge-to-edge IME overlays instead of resizing, so the frame height doesn't change on keyboard toggle and the PTY is never reflowed while typing; the refit's row-count guard makes sub-row jitter a no-op. * fix(mobile): guard height refit against IME resize; test the decision Address review on #8647: - Extract shouldRefitOnFrameHeightChange (pure) and gate the height refit on keyboard-visible, so an IME that resizes the window (Android adjustResize) can never reflow the PTY while typing — no longer relies on the edge-to-edge no-resize assumption alone. - Add a behavioral test for the decision helper (height transition, same-value no-op, keyboard-open skip) instead of only source-string assertions. - Trim the added comments to 1-2 lines per AGENTS.md. --- .../app/h/[hostId]/session/[worktreeId].tsx | 17 ++++++- .../terminal/terminal-viewport-refit-state.ts | 14 +++++ .../terminal/terminal-viewport-refit.test.ts | 51 ++++++++++++++++++- .../src/terminal/terminal-viewport-refit.ts | 32 +++++++++++- 4 files changed, 110 insertions(+), 4 deletions(-) diff --git a/mobile/app/h/[hostId]/session/[worktreeId].tsx b/mobile/app/h/[hostId]/session/[worktreeId].tsx index 5737683d481..5cc9ac912b7 100644 --- a/mobile/app/h/[hostId]/session/[worktreeId].tsx +++ b/mobile/app/h/[hostId]/session/[worktreeId].tsx @@ -1080,6 +1080,13 @@ export default function SessionScreen() { // the row) without any window-dim change. Tracking the measured width lets the // refit hook re-fit the PTY on those resizes — see terminal-viewport-refit.ts. const [terminalFrameWidth, setTerminalFrameWidth] = useState(0) + // Why: a new agent terminal can fit before the accessory/live-input dock lays + // out, over-fitting the PTY so its bottom-pinned input box hides behind the + // dock; tracking the settled height lets the refit hook correct it. + const [terminalFrameHeight, setTerminalFrameHeight] = useState(0) + // Why: lets the height refit skip keyboard-driven resizes (never reflow the + // PTY per keystroke); mirrors keyboardHeight but readable synchronously. + const keyboardVisibleRef = useRef(false) const activeSessionTab = sessionTabs.find((tab) => tab.id === activeSessionTabId) ?? null const { @@ -2602,15 +2609,19 @@ export default function SessionScreen() { tabStripVisible: terminals.length > 1, textScale: terminalTextScale, terminalFrameWidth, + terminalFrameHeight, + keyboardVisibleRef, unsubscribeTerminal, subscribeToTerminal }) useEffect(() => { const onShow = (e: KeyboardEvent) => { + keyboardVisibleRef.current = true setKeyboardHeight(e.endCoordinates?.height ?? 0) } const onHide = () => { + keyboardVisibleRef.current = false setKeyboardHeight(0) } const showEvent = Platform.OS === 'ios' ? 'keyboardWillShow' : 'keyboardDidShow' @@ -4831,10 +4842,12 @@ export default function SessionScreen() { style={styles.terminalFrame} onLayout={(e) => { terminalFrameHeightRef.current = e.nativeEvent.layout.height - // Trigger a refit only when the width actually changes (sidebar - // resize, fold, rotation) — avoids churn on height-only changes. + // Track width AND height so the refit hook re-fits on sidebar/ + // fold/rotation (width) and on the dock settling (height). const nextWidth = Math.round(e.nativeEvent.layout.width) + const nextHeight = Math.round(e.nativeEvent.layout.height) setTerminalFrameWidth((prev) => (prev === nextWidth ? prev : nextWidth)) + setTerminalFrameHeight((prev) => (prev === nextHeight ? prev : nextHeight)) }} > {terminals.map((terminal) => ( diff --git a/mobile/src/terminal/terminal-viewport-refit-state.ts b/mobile/src/terminal/terminal-viewport-refit-state.ts index 78da5c2fe06..e5787139fd9 100644 --- a/mobile/src/terminal/terminal-viewport-refit-state.ts +++ b/mobile/src/terminal/terminal-viewport-refit-state.ts @@ -24,6 +24,20 @@ export function isTerminalUpdateViewportApplied(response: RpcResponse): boolean return (response.result as { applied?: unknown }).applied === true } +// Why: re-fit when the flex-bounded frame height settles, but never while the +// keyboard is visible so an IME window resize (Android adjustResize) can't +// reflow the PTY per keystroke — it re-fits again once the keyboard closes. +export function shouldRefitOnFrameHeightChange(state: { + previousHeight: number + nextHeight: number + keyboardVisible: boolean +}): boolean { + if (state.keyboardVisible) { + return false + } + return state.previousHeight !== state.nextHeight +} + export function isTerminalViewportRefitTargetCurrent( state: TerminalViewportRefitTargetState ): boolean { diff --git a/mobile/src/terminal/terminal-viewport-refit.test.ts b/mobile/src/terminal/terminal-viewport-refit.test.ts index b658dad437a..54798a517e0 100644 --- a/mobile/src/terminal/terminal-viewport-refit.test.ts +++ b/mobile/src/terminal/terminal-viewport-refit.test.ts @@ -4,7 +4,8 @@ import type { RpcResponse } from '../transport/types' import { isTerminalUpdateViewportApplied, isTerminalUpdateViewportUpdated, - isTerminalViewportRefitTargetCurrent + isTerminalViewportRefitTargetCurrent, + shouldRefitOnFrameHeightChange } from './terminal-viewport-refit-state' const hookSource = readFileSync(new URL('./terminal-viewport-refit.ts', import.meta.url), 'utf8') @@ -48,11 +49,59 @@ describe('terminal viewport refit', () => { expect(textScaleEffect).toContain('[textScale, viewportMeasuredRef, scheduleViewportRefit]') }) + it('refits on a frame-height change only while the keyboard is closed', () => { + // The dock settling after a new agent terminal's first fit changes the frame + // height; refitting then stops the PTY over-fitting behind the dock. An IME + // that resizes the window (Android) must not reflow the PTY while typing. + // Height settled, keyboard closed → refit. + expect( + shouldRefitOnFrameHeightChange({ + previousHeight: 600, + nextHeight: 520, + keyboardVisible: false + }) + ).toBe(true) + // Unchanged height → no refit (don't churn on unrelated re-layouts). + expect( + shouldRefitOnFrameHeightChange({ + previousHeight: 520, + nextHeight: 520, + keyboardVisible: false + }) + ).toBe(false) + // Height changed while the keyboard is up → skip (never reflow while typing). + expect( + shouldRefitOnFrameHeightChange({ + previousHeight: 600, + nextHeight: 320, + keyboardVisible: true + }) + ).toBe(false) + }) + + it('routes the height effect through the keyboard-guarded decision helper', () => { + const start = hookSource.indexOf('const prevFrameHeightRef = useRef(terminalFrameHeight)') + expect(start).toBeGreaterThanOrEqual(0) + const heightEffect = hookSource.slice(start, start + 500) + expect(heightEffect).toContain('shouldRefitOnFrameHeightChange({') + expect(heightEffect).toContain('keyboardVisible: keyboardVisibleRef.current') + expect(heightEffect).toContain('viewportMeasuredRef.current = false') + expect(heightEffect).toContain('scheduleViewportRefit()') + }) + it('is wired into the session screen', () => { expect(sessionSource).toContain('useTerminalViewportRefit({') expect(sessionSource).toContain('tabStripVisible: terminals.length > 1') expect(sessionSource).toContain('textScale: terminalTextScale') expect(sessionSource).toContain('connState,') + // The session must feed the frame height and the keyboard-visible ref, or the + // guarded height effect never sees the dock settle / keyboard state. + expect(sessionSource).toContain('terminalFrameHeight,') + expect(sessionSource).toContain('keyboardVisibleRef,') + expect(sessionSource).toContain('keyboardVisibleRef.current = true') + expect(sessionSource).toContain( + 'setTerminalFrameHeight((prev) => (prev === nextHeight ? prev : nextHeight))' + ) }) it('forces a refit on iOS foreground and connection recovery', () => { diff --git a/mobile/src/terminal/terminal-viewport-refit.ts b/mobile/src/terminal/terminal-viewport-refit.ts index 44c6a9b7e03..e8b6aafd876 100644 --- a/mobile/src/terminal/terminal-viewport-refit.ts +++ b/mobile/src/terminal/terminal-viewport-refit.ts @@ -7,7 +7,8 @@ import { shouldRecoverTerminalOnAppStateChange } from './terminal-foreground-rec import { isTerminalUpdateViewportApplied, isTerminalUpdateViewportUpdated, - isTerminalViewportRefitTargetCurrent + isTerminalViewportRefitTargetCurrent, + shouldRefitOnFrameHeightChange } from './terminal-viewport-refit-state' export type TerminalViewportDims = { cols: number; rows: number } @@ -32,6 +33,12 @@ type TerminalViewportRefitOptions = { // tab-strip change. Carries that measured width so those resizes re-fit the PTY; // the 150ms debounce coalesces the stream of drag widths into one settle-time refit. terminalFrameWidth: number + // Why: the frame height settles when the accessory/live-input dock lays out + // after a new agent terminal's first fit; carrying it re-fits the PTY so its + // rows stop overflowing behind the dock. See shouldRefitOnFrameHeightChange. + terminalFrameHeight: number + // Why: gate the height refit so a keyboard-driven resize never reflows the PTY. + keyboardVisibleRef: RefObject unsubscribeTerminal: (handle: string) => void subscribeToTerminal: (handle: string) => void } @@ -56,6 +63,8 @@ export function useTerminalViewportRefit(options: TerminalViewportRefitOptions): tabStripVisible, textScale, terminalFrameWidth, + terminalFrameHeight, + keyboardVisibleRef, unsubscribeTerminal, subscribeToTerminal } = options @@ -226,6 +235,27 @@ export function useTerminalViewportRefit(options: TerminalViewportRefitOptions): scheduleViewportRefit() }, [terminalFrameWidth, viewportMeasuredRef, scheduleViewportRefit]) + // Why: re-fit when the frame height settles after a new agent terminal's + // first fit so its rows stop overflowing behind the dock; the keyboard guard + // keeps an IME resize from reflowing the PTY. The refit's row-count guard + // makes a same-row height change a no-op. + const prevFrameHeightRef = useRef(terminalFrameHeight) + useEffect(() => { + const previousHeight = prevFrameHeightRef.current + prevFrameHeightRef.current = terminalFrameHeight + if ( + !shouldRefitOnFrameHeightChange({ + previousHeight, + nextHeight: terminalFrameHeight, + keyboardVisible: keyboardVisibleRef.current + }) + ) { + return + } + viewportMeasuredRef.current = false + scheduleViewportRefit() + }, [terminalFrameHeight, keyboardVisibleRef, viewportMeasuredRef, scheduleViewportRefit]) + useEffect(() => { if (Platform.OS !== 'ios') { return