diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index 1e244518..6ca4a001 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -653,6 +653,7 @@ Object.assign(CodemanApp.prototype, { let didScroll = false; // track whether touchmove fired (tap vs scroll) let touchStartY = 0; let tapStartedWithTerminalFocus = false; + let tapStartIntentCache = null; const TAP_THRESHOLD = 8; // px — ignore micro-drift to distinguish tap from scroll container.addEventListener( 'touchstart', @@ -666,11 +667,24 @@ Object.assign(CodemanApp.prototype, { isTouching = true; didScroll = false; tapStartedWithTerminalFocus = this._isMobileTerminalInputFocused(); + // Classifying scans the whole viewport with translateToString, and + // this runs at the start of EVERY gesture including scroll drags. + // Cache the result for the touchend of this same gesture rather than + // recomputing it; the cache is keyed on the exact start coordinates + // so a finger that moved re-classifies at its real position. const touchStartIntent = this._classifyMobileTerminalTap(touchLastX, touchLastY); - if (touchStartIntent !== 'input') { + tapStartIntentCache = { x: touchLastX, y: touchLastY, intent: touchStartIntent }; + if (touchStartIntent === 'content') { // Cancel xterm/browser focus before the compatibility click can // open the OS keyboard. Content taps are re-emitted as SGR on - // touchend; history taps deliberately remain inert. + // touchend. + // + // 'history' is deliberately NOT included. A scrolled-up viewport + // sends nothing, so there is no compatibility click worth + // cancelling — and preventDefault() here, paired with touchend's + // early return, closes both routes to focus at once. Since + // selectSession() ends with scrollToLastNonEmptyLine(), that made + // the keyboard unreachable after every tab switch. ev.preventDefault(); this._blurMobileTerminalInput(); } @@ -700,7 +714,6 @@ Object.assign(CodemanApp.prototype, { // fling, so a jittery tap would both position the cursor AND scroll. if (!didScroll) return; ev.preventDefault(); - touchLastX = ev.touches[0].clientX; const delta = touchLastY - touchY; // positive = scroll down pixelAccum += delta; velocity = delta * 1.2; @@ -737,7 +750,13 @@ Object.assign(CodemanApp.prototype, { const touch = ev.changedTouches && ev.changedTouches[0]; if (touch) { this._suppressTrustedTapMouseEvents(); - this._handleMobileTerminalTap(touch, tapStartedWithTerminalFocus); + const cached = + tapStartIntentCache && + tapStartIntentCache.x === touch.clientX && + tapStartIntentCache.y === touch.clientY + ? tapStartIntentCache.intent + : null; + this._handleMobileTerminalTap(touch, tapStartedWithTerminalFocus, cached); } } tapStartedWithTerminalFocus = false; @@ -3371,9 +3390,6 @@ Object.assign(CodemanApp.prototype, { const mouseTrackingOn = !!mouseMode && mouseMode !== 'none'; if (!mouseTrackingOn && !this._sessionUsesServerMouseStrip()) return 'input'; - // Permission/elicitation prompts own the full live terminal until answered. - if (document.body?.classList?.contains('terminal-action-pending')) return 'content'; - const buffer = this.terminal.buffer?.active; if (!buffer?.getLine) return 'input'; @@ -3421,10 +3437,13 @@ Object.assign(CodemanApp.prototype, { let logicalLineEnd = tappedRow; while (logicalLineEnd + 1 < rows && wrappedRows[logicalLineEnd + 1]) logicalLineEnd++; const tappedLine = lines.slice(logicalLineStart, logicalLineEnd + 1).join(''); - if ( - mode === 'claude' && - /^\s*[•·]\s*Working\b.*(?:background|esc to interrupt)/i.test(tappedLine) - ) { + // Claude's status row is TUI-owned: tapping it opens the teammate view, so it + // must not be treated as a keyboard target. Match the AFFORDANCE, not the + // wording — the bullet and verb are both unstable (claude 2.1.226 prints + // "✻ Cooked for 2m 6s", "✻ Baked for 9m 47s"; earlier builds printed + // "• Working …"), while "esc to interrupt" / "background" are what make the + // row actionable in the first place. + if (mode === 'claude' && /\b(?:esc to interrupt|background)\b/i.test(tappedLine)) { return 'content'; } if (menuSelectionVisible) return 'content'; @@ -3471,8 +3490,6 @@ Object.assign(CodemanApp.prototype, { * otherwise focus xterm, so focus has to be restored explicitly. */ _isActionableMobileTerminalTap(clientX, clientY) { - if (document.body?.classList?.contains('terminal-action-pending')) return true; - const pos = this._clientPointToCell(clientX, clientY); const buffer = this.terminal?.buffer?.active; if (!pos || !buffer?.getLine) return false; @@ -3502,14 +3519,22 @@ Object.assign(CodemanApp.prototype, { // The hint sits on its own row, so a readback's TITLE row — the one a // finger actually lands on — carries no affordance text itself. Look at the // adjacent row too, which is how these blocks are laid out in practice. + // Keyed on the ACTION VERB, and deliberately not on prose verbs. A CLI hint + // names a key or a gesture ("ctrl+r to expand", "tap to collapse", + // "esc to interrupt"); "click here to open the file" is transcript content + // and must keep the keyboard, so `click` and bare `here` are excluded. + // The hint may sit mid-line — Claude's status row is + // "✻ Cooked for 2m 6s · esc to interrupt" — so this is not anchored. const affordance = - /\b(?:ctrl\+\w+|tap|click|enter|esc)\b[^.]{0,24}\bto\s+(?:expand|collapse|view|open|interrupt|see)\b/i; + /\b(?:ctrl\+\w+|shift\+\w+|esc|enter|tab|tap)\s+to\s+(?:expand|collapse|view|open|interrupt|see)\b/i; const blockStart = Math.max(0, logicalLineStart - 1); const blockEnd = Math.min(rows - 1, logicalLineEnd + 1); for (let row = blockStart; row <= blockEnd; row++) { if (affordance.test(lines[row])) return true; } - if (/^\s*[•·]\s*Working\b/i.test(tappedLine)) return true; + // A Claude status row ("✻ Cooked for 2m 6s · esc to interrupt") is caught by + // the affordance above; there is deliberately no verb literal here, because + // the verb is randomised per build. const hasMenuPrompt = lines.some((line) => /^\s*[❯›]\s+\d+[.)]\s/.test(line)); const hasMenuChoice = lines.some((line) => /^\s+\d+[.)]\s/.test(line)); @@ -3526,11 +3551,20 @@ Object.assign(CodemanApp.prototype, { } }, - _handleMobileTerminalTap(touch, startedWithTerminalFocus) { - if (!touch || !this.terminal) return 'history'; - const intent = this._classifyMobileTerminalTap(touch.clientX, touch.clientY); + _handleMobileTerminalTap(touch, startedWithTerminalFocus, cachedIntent = null) { + // A guard bail-out, not a classification: there is nothing to classify. It is + // deliberately NOT 'history', which would claim the viewport was scrolled up. + if (!touch || !this.terminal) return null; + // touchstart already classified this exact point; reuse it rather than paying + // a second full-viewport scan for the same gesture. + const intent = cachedIntent ?? this._classifyMobileTerminalTap(touch.clientX, touch.clientY); if (intent === 'history') { - this._blurMobileTerminalInput(); + // Scrolled up: send NO mouse report — a tap on old output must not be + // delivered to the CLI as a click on whatever row now occupies that cell. + // Focus is a separate question, and the answer is yes: the user tapped the + // terminal, so let them type. Blurring here stranded activeElement on + // with no way back to the keyboard. + this._focusMobileTerminalInput(); return intent; } @@ -3740,15 +3774,6 @@ Object.assign(CodemanApp.prototype, { return true; }, - // Claude keeps most transcript history inside its own TUI rather than xterm - // scrollback. On verified versions, route a touch drag through the same SGR - // wheel path as desktop. Codex keeps the existing local touch behavior. - _shouldForwardTouchScrollToApp() { - const session = this.sessions?.get(this.activeSessionId); - if (session?.mode !== 'claude') return false; - return this._shouldForwardWheelToApp({ shiftKey: false }); - }, - // Encode wheel ticks as SGR reports (button 64 = up, 65 = down) at the pointer // cell. Reports are coalesced into one fire-and-forget write per ~40ms: a // trackpad emits dozens of wheel events per second and each send becomes a diff --git a/test/mobile/keyboard.test.ts b/test/mobile/keyboard.test.ts index e583c21b..df0c4cd1 100644 --- a/test/mobile/keyboard.test.ts +++ b/test/mobile/keyboard.test.ts @@ -756,6 +756,94 @@ describe('Virtual Keyboard', () => { it('focuses the terminal helper textarea when the terminal is tapped', async () => { await page.evaluate(() => { + app.activeSessionId = 'mobile-focus-visible-input-test'; + app.sessions.set('mobile-focus-visible-input-test', { + id: 'mobile-focus-visible-input-test', + mode: 'codex', + status: 'running', + }); + app.hideWelcome(); + const settings = app.loadAppSettingsFromStorage(); + settings.cjkInputEnabled = false; + app.saveAppSettingsToStorage(settings); + app._updateCjkInputState(); + }); + + await page.locator('#terminalContainer').tap({ position: { x: 40, y: 40 } }); + + const activeClass = await page.evaluate(() => document.activeElement?.className); + expect(activeClass).toContain('xterm-helper-textarea'); + }); + + // Regression guard for the phone-keyboard blocker reduced in #173 and re-hit + // by #244. selectSession() ends with scrollToLastNonEmptyLine(), which parks + // the viewport ABOVE the bottom for any session whose buffer is taller than + // the screen and ends in blank rows, i.e. every real session after a tab + // switch. A tap-routing scheme that treats "viewport is scrolled up" as a + // reason to blur strands document.activeElement on with no way to + // raise the keyboard, and the prompt row is no exception. Suppressing the + // MOUSE REPORT while scrolled up is correct and pinned below; suppressing + // FOCUS is not. Measured against PR #244 on 2026-08-09: body vs textarea. + // + // Must be a dispatched gesture: calling the touchend handler directly + // bypasses touchstart's preventDefault, which is half of what closes the + // focus path, so a direct call reports the right intent and still misses. + it('keeps the terminal input focusable after a tab switch parks the viewport off-bottom', async () => { + const probe = await page.evaluate(async () => { + window.__sentInputs = []; + app.activeSessionId = 'mobile-offbottom-tap-test'; + app.sessions.set('mobile-offbottom-tap-test', { + id: 'mobile-offbottom-tap-test', + mode: 'claude', + cliVersion: '2.1.220', + status: 'running', + }); + app._sendInputAsync = (_sessionId: string, input: string) => { + window.__sentInputs.push(input); + }; + app.hideWelcome(); + const settings = app.loadAppSettingsFromStorage(); + settings.cjkInputEnabled = false; + app.saveAppSettingsToStorage(settings); + app._updateCjkInputState(); + app.terminal.reset(); + + // Taller than the viewport, ending in the trailing blank rows that make + // scrollToLastNonEmptyLine() stop short of the bottom. + const lines: string[] = []; + for (let i = 1; i <= app.terminal.rows * 3; i++) lines.push(`Transcript row ${i}`); + lines.push('', '❯ ', '', ''); + await new Promise((resolve) => app.terminal.write(lines.join('\r\n'), resolve)); + + app.scrollToLastNonEmptyLine(); // what selectSession() does on every tab switch + (document.activeElement as HTMLElement | null)?.blur?.(); + + const screen = app.terminal.element?.querySelector('.xterm-screen'); + const cell = app.terminal._core?._renderService?.dimensions?.css?.cell; + const rect = screen?.getBoundingClientRect(); + if (!rect || !cell?.width || !cell?.height) return null; + const buffer = app.terminal.buffer.active; + return { + x: rect.left + cell.width * 2, + y: rect.top + cell.height * 5.5, + atBottom: buffer.viewportY >= buffer.baseY, + }; + }); + + expect(probe).not.toBeNull(); + // The guard only means anything if the viewport really did park off-bottom. + expect(probe!.atBottom).toBe(false); + + await page.touchscreen.tap(probe!.x, probe!.y); + + const state = await page.evaluate(() => ({ + activeClass: document.activeElement?.className, + sentInputs: window.__sentInputs, + })); + expect(state.activeClass).toContain('xterm-helper-textarea'); + // SGR coordinates are meaningless off-bottom, so the tap must stay silent. + expect(state.sentInputs).toEqual([]); + }); it('collapses a terminal readback without focusing the hidden textarea', async () => { const point = await page.evaluate(async () => { @@ -961,42 +1049,6 @@ describe('Virtual Keyboard', () => { mode: 'codex', status: 'running', }); - app.hideWelcome(); - const settings = app.loadAppSettingsFromStorage(); - settings.cjkInputEnabled = false; - app.saveAppSettingsToStorage(settings); - app._updateCjkInputState(); - }); - - await page.locator('#terminalContainer').tap({ position: { x: 40, y: 40 } }); - - const activeClass = await page.evaluate(() => document.activeElement?.className); - expect(activeClass).toContain('xterm-helper-textarea'); - }); - - // Regression guard for the phone-keyboard blocker reduced in #173 and re-hit - // by #244. selectSession() ends with scrollToLastNonEmptyLine(), which parks - // the viewport ABOVE the bottom for any session whose buffer is taller than - // the screen and ends in blank rows, i.e. every real session after a tab - // switch. A tap-routing scheme that treats "viewport is scrolled up" as a - // reason to blur strands document.activeElement on with no way to - // raise the keyboard, and the prompt row is no exception. Suppressing the - // MOUSE REPORT while scrolled up is correct and pinned below; suppressing - // FOCUS is not. Measured against PR #244 on 2026-08-09: body vs textarea. - // - // Must be a dispatched gesture: calling the touchend handler directly - // bypasses touchstart's preventDefault, which is half of what closes the - // focus path, so a direct call reports the right intent and still misses. - it('keeps the terminal input focusable after a tab switch parks the viewport off-bottom', async () => { - const probe = await page.evaluate(async () => { - window.__sentInputs = []; - app.activeSessionId = 'mobile-offbottom-tap-test'; - app.sessions.set('mobile-offbottom-tap-test', { - id: 'mobile-offbottom-tap-test', - mode: 'claude', - cliVersion: '2.1.220', - status: 'running', - }); app._sendInputAsync = (_sessionId: string, input: string) => { window.__sentInputs.push(input); }; @@ -1006,41 +1058,29 @@ describe('Virtual Keyboard', () => { app.saveAppSettingsToStorage(settings); app._updateCjkInputState(); app.terminal.reset(); - - // Taller than the viewport, ending in the trailing blank rows that make - // scrollToLastNonEmptyLine() stop short of the bottom. - const lines: string[] = []; - for (let i = 1; i <= app.terminal.rows * 3; i++) lines.push(`Transcript row ${i}`); - lines.push('', '❯ ', '', ''); - await new Promise((resolve) => app.terminal.write(lines.join('\r\n'), resolve)); - - app.scrollToLastNonEmptyLine(); // what selectSession() does on every tab switch + await new Promise((resolve) => + app.terminal.write('Agent readback\r\n tap to collapse\r\n\r\n› ask', resolve) + ); (document.activeElement as HTMLElement | null)?.blur?.(); const screen = app.terminal.element?.querySelector('.xterm-screen'); const cell = app.terminal._core?._renderService?.dimensions?.css?.cell; const rect = screen?.getBoundingClientRect(); if (!rect || !cell?.width || !cell?.height) return null; - const buffer = app.terminal.buffer.active; return { x: rect.left + cell.width * 2, - y: rect.top + cell.height * 5.5, - atBottom: buffer.viewportY >= buffer.baseY, + y: rect.top + cell.height * (app.terminal.buffer.active.cursorY + 0.5), }; }); + expect(point).not.toBeNull(); - expect(probe).not.toBeNull(); - // The guard only means anything if the viewport really did park off-bottom. - expect(probe!.atBottom).toBe(false); - - await page.touchscreen.tap(probe!.x, probe!.y); + await page.touchscreen.tap(point!.x, point!.y); const state = await page.evaluate(() => ({ activeClass: document.activeElement?.className, sentInputs: window.__sentInputs, })); expect(state.activeClass).toContain('xterm-helper-textarea'); - // SGR coordinates are meaningless off-bottom, so the tap must stay silent. expect(state.sentInputs).toEqual([]); }); diff --git a/test/terminal-touch-tap.test.ts b/test/terminal-touch-tap.test.ts index e1139038..8562871f 100644 --- a/test/terminal-touch-tap.test.ts +++ b/test/terminal-touch-tap.test.ts @@ -543,25 +543,6 @@ describe('terminal touch tap mouse guard', () => { expect(app._shouldForwardWheelToApp({ shiftKey: false })).toBe(false); }); - it('touch: forwards verified Claude transcript scrolling but keeps Codex touch in local history', () => { - const { app } = loadTerminalUiHarness(); - app.activeSessionId = 'sess-1'; - app.terminal = { - modes: { mouseTrackingMode: 'none' }, - buffer: { active: { viewportY: 50, baseY: 50 } }, - }; - - app.sessions = new Map([['sess-1', { mode: 'claude', cliVersion: '2.1.220' }]]); - expect(app._shouldForwardTouchScrollToApp()).toBe(true); - - app.loadAppSettingsFromStorage = () => ({ terminalWheelLocalScrollback: true }); - expect(app._shouldForwardTouchScrollToApp()).toBe(false); - - app.loadAppSettingsFromStorage = () => ({ terminalWheelLocalScrollback: false }); - app.sessions = new Map([['sess-1', { mode: 'codex' }]]); - expect(app._shouldForwardTouchScrollToApp()).toBe(false); - }); - it('wheel: the local-scrollback opt-out pins the plain wheel to local scrollback (issue #154)', () => { const { app } = loadTerminalUiHarness(); app.activeSessionId = 'sess-1';