From 66abe6c70a6203a140cf79e2ba98e904d445c6c8 Mon Sep 17 00:00:00 2001 From: lior Date: Wed, 29 Jul 2026 12:17:39 +0300 Subject: [PATCH 1/3] fix(mobile): coalesce keyboard viewport settling --- src/web/public/mobile-handlers.js | 75 ++++++++++++++++--------------- test/mobile/keyboard.test.ts | 38 ++++++++++++++++ 2 files changed, 76 insertions(+), 37 deletions(-) diff --git a/src/web/public/mobile-handlers.js b/src/web/public/mobile-handlers.js index 5b8b99a5..ea1ed8f8 100644 --- a/src/web/public/mobile-handlers.js +++ b/src/web/public/mobile-handlers.js @@ -209,9 +209,12 @@ const MobileDetection = { * Also handles terminal scrolling and toolbar repositioning via visualViewport API. */ const KeyboardHandler = { + VIEWPORT_SETTLE_MS: 80, lastViewportHeight: 0, keyboardVisible: false, initialViewportHeight: 0, + _viewportSettleTimer: null, + _settleScrollToBottom: false, /** Initialize keyboard handling */ init() { @@ -276,6 +279,11 @@ const KeyboardHandler = { window.removeEventListener('scroll', this._windowScrollHandler); this._windowScrollHandler = null; } + if (this._viewportSettleTimer) { + clearTimeout(this._viewportSettleTimer); + this._viewportSettleTimer = null; + } + this._settleScrollToBottom = false; }, /** Handle viewport resize (keyboard show/hide) */ @@ -313,6 +321,7 @@ const KeyboardHandler = { } this.updateLayoutForKeyboard(); + this._scheduleViewportSettle(); this.lastViewportHeight = currentHeight; }, @@ -414,32 +423,9 @@ const KeyboardHandler = { // iOS Safari may scroll the document to reveal xterm's hidden textarea. window.scrollTo(0, 0); - // Refit terminal locally AND send resize to server so Claude Code (Ink) - // knows the actual terminal dimensions. Without this, Ink redraws at the - // old (larger) row count when the user types, causing content to scroll - // off the visible area with each keystroke. - // Note: the throttledResize handler still suppresses ongoing resize events - // while keyboard is up — this one-shot resize on open/close is sufficient. - setTimeout(() => { - if (typeof app !== 'undefined' && app.terminal) { - if (app.fitAddon) - try { - app.fitAddon.fit(); - } catch {} - // Eliminate terminal row quantization gap: xterm can only show whole - // rows, so leftover pixels create dead space below the last row. - // Shrink .main's paddingBottom by the gap so the terminal fills flush - // to the accessory bar. - this._shrinkPaddingToFit(); - app.terminal.scrollToBottom(); - app._syncMobileHelperTextareaToCursor?.(); - app._localEchoOverlay?.rerender?.(); - // Send resize to server so PTY dimensions match xterm - this._sendTerminalResize(); - } - // Reset again after fit/resize in case layout changes triggered scroll - window.scrollTo(0, 0); - }, 150); + // visualViewport emits multiple heights throughout the OS animation. + // Re-schedule on every event and fit only after the final height settles. + this._scheduleViewportSettle({ scrollToBottom: true }); // Reposition subagent windows to stack from bottom (above keyboard) if (typeof app !== 'undefined') app.relayoutMobileSubagentWindows(); @@ -454,22 +440,37 @@ const KeyboardHandler = { this.resetLayout(); - // Refit terminal, scroll to bottom, and send resize to restore original dimensions - setTimeout(() => { - if (typeof app !== 'undefined' && app.fitAddon) { - try { - app.fitAddon.fit(); - } catch {} - if (app.terminal) app.terminal.scrollToBottom(); - // Send resize to server to restore full terminal size - this._sendTerminalResize(); - } - }, 100); + this._scheduleViewportSettle({ scrollToBottom: true }); // Reposition subagent windows to stack from top (below header) if (typeof app !== 'undefined') app.relayoutMobileSubagentWindows(); }, + /** Coalesce the keyboard animation into one final xterm reflow and PTY resize. */ + _scheduleViewportSettle({ scrollToBottom = false } = {}) { + this._settleScrollToBottom = this._settleScrollToBottom || scrollToBottom; + if (this._viewportSettleTimer) clearTimeout(this._viewportSettleTimer); + this._viewportSettleTimer = setTimeout(() => { + this._viewportSettleTimer = null; + const shouldScrollToBottom = this._settleScrollToBottom; + this._settleScrollToBottom = false; + + if (typeof app !== 'undefined' && app.terminal) { + if (app.fitAddon) { + try { + app.fitAddon.fit(); + } catch {} + } + if (this.keyboardVisible) this._shrinkPaddingToFit(); + if (shouldScrollToBottom) app.terminal.scrollToBottom(); + app._syncMobileHelperTextareaToCursor?.(); + app._localEchoOverlay?.rerender?.(); + this._sendTerminalResize(); + } + window.scrollTo(0, 0); + }, this.VIEWPORT_SETTLE_MS); + }, + /** Send current terminal dimensions to the server (one-shot, for keyboard open/close) */ _sendTerminalResize() { if (typeof app === 'undefined' || !app.activeSessionId || !app.fitAddon) return; diff --git a/test/mobile/keyboard.test.ts b/test/mobile/keyboard.test.ts index 5032f653..a99ae023 100644 --- a/test/mobile/keyboard.test.ts +++ b/test/mobile/keyboard.test.ts @@ -323,6 +323,44 @@ describe('Virtual Keyboard', () => { expect(mainPadding).toBe(''); }); + it('coalesces keyboard animation frames into one final terminal fit', async () => { + const result = await page.evaluate(async () => { + const originalFit = app.fitAddon.fit.bind(app.fitAddon); + const originalSendResize = KeyboardHandler._sendTerminalResize.bind(KeyboardHandler); + const originalScrollToBottom = app.terminal.scrollToBottom.bind(app.terminal); + let fits = 0; + let resizes = 0; + let bottomRestores = 0; + app.fitAddon.fit = () => { + fits++; + }; + KeyboardHandler._sendTerminalResize = () => { + resizes++; + }; + app.terminal.scrollToBottom = () => { + bottomRestores++; + }; + + KeyboardHandler._scheduleViewportSettle({ scrollToBottom: true }); + await new Promise((resolve) => setTimeout(resolve, 30)); + KeyboardHandler._scheduleViewportSettle(); + await new Promise((resolve) => setTimeout(resolve, 30)); + KeyboardHandler._scheduleViewportSettle(); + await new Promise((resolve) => setTimeout(resolve, 50)); + const beforeFinalSettle = { fits, resizes, bottomRestores }; + await new Promise((resolve) => setTimeout(resolve, KeyboardHandler.VIEWPORT_SETTLE_MS)); + const afterFinalSettle = { fits, resizes, bottomRestores }; + + app.fitAddon.fit = originalFit; + KeyboardHandler._sendTerminalResize = originalSendResize; + app.terminal.scrollToBottom = originalScrollToBottom; + return { beforeFinalSettle, afterFinalSettle }; + }); + + expect(result.beforeFinalSettle).toEqual({ fits: 0, resizes: 0, bottomRestores: 0 }); + expect(result.afterFinalSettle).toEqual({ fits: 1, resizes: 1, bottomRestores: 1 }); + }); + it('accessory bar has the simple-mode action buttons', async () => { const actions = await page.evaluate(() => { return Array.from(document.querySelectorAll('.keyboard-accessory-bar [data-action]')).map( From 0a1439b1e9c6be6bea19f0de8083aff4c7cfd150 Mon Sep 17 00:00:00 2001 From: lior Date: Sat, 8 Aug 2026 08:41:04 +0300 Subject: [PATCH 2/3] test(mobile): make the coalescing test actually exercise the settle path The suite never selects a session, so initTerminal() does not run and both `app.terminal` and `app.fitAddon` are null at rest. `_scheduleViewportSettle` returns early on a falsy terminal, so the coalescing assertions could not reach the behavior they claimed to cover -- the test errored on `Cannot read properties of null` rather than measuring anything. Installs the minimum surface the settle callback touches and restores it afterwards, so the coalescing path executes for real. Adds a behavioral counterpart driven through the PUBLIC entry point (`onKeyboardShow`) instead of the internal scheduler: three viewport steps in quick succession must produce exactly ONE refit. On master that returns 3 (each show arms its own uncoalesced 150ms timeout), so this fails by COUNT rather than by a missing method -- which is the failure mode that actually demonstrates the bug. Verified: `expected 3 to be 1` on unmodified master; passes here. The remaining 8 failures in this file are pre-existing on master and unrelated (same null-initialization limitation of the headless harness). --- test/mobile/keyboard.test.ts | 56 ++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/test/mobile/keyboard.test.ts b/test/mobile/keyboard.test.ts index a99ae023..721b9da3 100644 --- a/test/mobile/keyboard.test.ts +++ b/test/mobile/keyboard.test.ts @@ -325,6 +325,16 @@ describe('Virtual Keyboard', () => { it('coalesces keyboard animation frames into one final terminal fit', async () => { const result = await page.evaluate(async () => { + // `app.terminal` and `app.fitAddon` are only assigned by initTerminal(), + // which needs a selected session this harness never creates. Both are + // null at rest, and the settle callback returns early on a falsy + // terminal — so without stand-ins this test cannot reach the behavior + // it asserts. Install the minimum surface the callback touches. + const hadTerminal = app.terminal !== null && app.terminal !== undefined; + const hadFitAddon = app.fitAddon !== null && app.fitAddon !== undefined; + if (!hadTerminal) app.terminal = { scrollToBottom() {} }; + if (!hadFitAddon) app.fitAddon = { fit() {}, proposeDimensions: () => null }; + const originalFit = app.fitAddon.fit.bind(app.fitAddon); const originalSendResize = KeyboardHandler._sendTerminalResize.bind(KeyboardHandler); const originalScrollToBottom = app.terminal.scrollToBottom.bind(app.terminal); @@ -354,6 +364,8 @@ describe('Virtual Keyboard', () => { app.fitAddon.fit = originalFit; KeyboardHandler._sendTerminalResize = originalSendResize; app.terminal.scrollToBottom = originalScrollToBottom; + if (!hadFitAddon) app.fitAddon = null; + if (!hadTerminal) app.terminal = null; return { beforeFinalSettle, afterFinalSettle }; }); @@ -361,6 +373,50 @@ describe('Virtual Keyboard', () => { expect(result.afterFinalSettle).toEqual({ fits: 1, resizes: 1, bottomRestores: 1 }); }); + // Behavioral counterpart to the test above, driven through the PUBLIC entry + // point rather than the internal scheduler. Before this change each + // onKeyboardShow armed its own uncoalesced 150ms setTimeout, so a keyboard + // animation that reports several viewport steps refit the terminal once per + // step — the visible symptom being repeated reflow while the keyboard slides + // up. This asserts the observable outcome (one refit for a burst) and so + // fails on master by COUNT, not by a missing method. + it('refits once for a burst of keyboard viewport steps', async () => { + const counts = await page.evaluate(async () => { + const hadTerminal = app.terminal !== null && app.terminal !== undefined; + const hadFitAddon = app.fitAddon !== null && app.fitAddon !== undefined; + if (!hadTerminal) app.terminal = { scrollToBottom() {} }; + if (!hadFitAddon) app.fitAddon = { fit() {}, proposeDimensions: () => null }; + + const originalFit = app.fitAddon.fit.bind(app.fitAddon); + const originalSendResize = KeyboardHandler._sendTerminalResize.bind(KeyboardHandler); + let fits = 0; + app.fitAddon.fit = () => { + fits++; + }; + KeyboardHandler._sendTerminalResize = () => {}; + + // Three viewport steps in quick succession, as a keyboard animation + // produces on a real device. + KeyboardHandler.onKeyboardShow(); + await new Promise((resolve) => setTimeout(resolve, 30)); + KeyboardHandler.onKeyboardShow(); + await new Promise((resolve) => setTimeout(resolve, 30)); + KeyboardHandler.onKeyboardShow(); + + // Well past both the coalescing window and master's fixed 150ms timer. + await new Promise((resolve) => setTimeout(resolve, 400)); + + app.fitAddon.fit = originalFit; + KeyboardHandler._sendTerminalResize = originalSendResize; + if (!hadFitAddon) app.fitAddon = null; + if (!hadTerminal) app.terminal = null; + return fits; + }); + + // Coalesced: one refit for the whole burst. Master fires one per step. + expect(counts).toBe(1); + }); + it('accessory bar has the simple-mode action buttons', async () => { const actions = await page.evaluate(() => { return Array.from(document.querySelectorAll('.keyboard-accessory-bar [data-action]')).map( From 1f164bc8d271588252120767276c1c8df907db40 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Sat, 8 Aug 2026 12:06:08 +0200 Subject: [PATCH 3/3] fix(mobile): only arm the viewport settle on a real keyboard transition A visualViewport resize event without a pending show/hide transition now only pushes a pending settle back (_deferViewportSettle) instead of arming fit + PTY-resize work of its own. Keyboard detection can miss a fine-grained OS animation entirely (each step under 150px, with the baseline chasing the animation down), while MobileDetection's own listener still shrinks --app-height, so the per-event settle fitted xterm against a mid-animation container with no keyboard CSS compensation and resized the PTY to transient dims. The resulting SIGWINCH thrash (58 -> 10 -> 50 rows) duplicated prompts and left tmux dot filler in the transcript on keyboard close. Reproduced with a faked visualViewport driving the real handler; master is unaffected because it never resized the PTY from this path. Co-Authored-By: Claude Fable 5 --- src/web/public/mobile-handlers.js | 28 +++++++++++++++++-- test/mobile/keyboard.test.ts | 46 +++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/src/web/public/mobile-handlers.js b/src/web/public/mobile-handlers.js index ea1ed8f8..0cc7e860 100644 --- a/src/web/public/mobile-handlers.js +++ b/src/web/public/mobile-handlers.js @@ -215,6 +215,7 @@ const KeyboardHandler = { initialViewportHeight: 0, _viewportSettleTimer: null, _settleScrollToBottom: false, + _settlePending: false, /** Initialize keyboard handling */ init() { @@ -284,6 +285,7 @@ const KeyboardHandler = { this._viewportSettleTimer = null; } this._settleScrollToBottom = false; + this._settlePending = false; }, /** Handle viewport resize (keyboard show/hide) */ @@ -321,7 +323,7 @@ const KeyboardHandler = { } this.updateLayoutForKeyboard(); - this._scheduleViewportSettle(); + this._deferViewportSettle(); this.lastViewportHeight = currentHeight; }, @@ -446,12 +448,34 @@ const KeyboardHandler = { if (typeof app !== 'undefined') app.relayoutMobileSubagentWindows(); }, - /** Coalesce the keyboard animation into one final xterm reflow and PTY resize. */ + /** + * Coalesce the keyboard animation into one final xterm reflow and PTY resize. + * Only a real show/hide transition arms the settle work; ongoing viewport + * resize events merely push a pending settle back (_deferViewportSettle). + * A viewport change that never crosses the show/hide thresholds must not + * refit: keyboard detection can miss a fine-grained OS animation entirely + * (each step under 150px, with the baseline chasing the animation), and the + * container is then mid-animation with no keyboard CSS compensation, so a + * fit against it resizes the PTY to transient dims and the SIGWINCH thrash + * garbles the transcript. + */ _scheduleViewportSettle({ scrollToBottom = false } = {}) { this._settleScrollToBottom = this._settleScrollToBottom || scrollToBottom; + this._settlePending = true; + this._armViewportSettleTimer(); + }, + + /** Push a pending settle back while the viewport is still animating; no-op otherwise. */ + _deferViewportSettle() { + if (!this._settlePending) return; + this._armViewportSettleTimer(); + }, + + _armViewportSettleTimer() { if (this._viewportSettleTimer) clearTimeout(this._viewportSettleTimer); this._viewportSettleTimer = setTimeout(() => { this._viewportSettleTimer = null; + this._settlePending = false; const shouldScrollToBottom = this._settleScrollToBottom; this._settleScrollToBottom = false; diff --git a/test/mobile/keyboard.test.ts b/test/mobile/keyboard.test.ts index 721b9da3..f17cd104 100644 --- a/test/mobile/keyboard.test.ts +++ b/test/mobile/keyboard.test.ts @@ -417,6 +417,52 @@ describe('Virtual Keyboard', () => { expect(counts).toBe(1); }); + // A viewport resize with NO pending show/hide transition must not arm settle + // work of its own: keyboard detection can miss a fine-grained OS animation + // entirely (sub-150px steps with the baseline chasing the animation), and a + // fit against that mid-animation, uncompensated layout resizes the PTY to + // transient dims. The resulting SIGWINCH thrash duplicates prompts and + // garbles the transcript. Wiggles may only push a pending settle back. + it('does not refit on viewport wiggles without a keyboard transition', async () => { + const result = await page.evaluate(async () => { + const hadTerminal = app.terminal !== null && app.terminal !== undefined; + const hadFitAddon = app.fitAddon !== null && app.fitAddon !== undefined; + if (!hadTerminal) app.terminal = { scrollToBottom() {} }; + if (!hadFitAddon) app.fitAddon = { fit() {}, proposeDimensions: () => null }; + + const originalFit = app.fitAddon.fit.bind(app.fitAddon); + const originalSendResize = KeyboardHandler._sendTerminalResize.bind(KeyboardHandler); + let fits = 0; + app.fitAddon.fit = () => { + fits++; + }; + KeyboardHandler._sendTerminalResize = () => {}; + + // Wiggle only: nothing pending, so nothing may fire. + KeyboardHandler._deferViewportSettle(); + KeyboardHandler._deferViewportSettle(); + await new Promise((resolve) => setTimeout(resolve, KeyboardHandler.VIEWPORT_SETTLE_MS + 80)); + const wiggleOnly = fits; + + // A real transition arms the work; a following wiggle defers it but the + // settle still fires exactly once. + KeyboardHandler._scheduleViewportSettle({ scrollToBottom: true }); + await new Promise((resolve) => setTimeout(resolve, 30)); + KeyboardHandler._deferViewportSettle(); + await new Promise((resolve) => setTimeout(resolve, KeyboardHandler.VIEWPORT_SETTLE_MS + 80)); + const afterTransition = fits; + + app.fitAddon.fit = originalFit; + KeyboardHandler._sendTerminalResize = originalSendResize; + if (!hadFitAddon) app.fitAddon = null; + if (!hadTerminal) app.terminal = null; + return { wiggleOnly, afterTransition }; + }); + + expect(result.wiggleOnly).toBe(0); + expect(result.afterTransition).toBe(1); + }); + it('accessory bar has the simple-mode action buttons', async () => { const actions = await page.evaluate(() => { return Array.from(document.querySelectorAll('.keyboard-accessory-bar [data-action]')).map(