From 6b729bd3480a7716d44850e906ec72089b6e2f7a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 17 Apr 2026 07:54:27 +0000 Subject: [PATCH 1/2] feat(web): enable terminal input on mobile browsers The web terminal previously blocked input on touch devices via `disableStdin: mobile`, gated `terminal.onData` behind `!mobile`, and installed a touchstart preventDefault that stopped xterm's hidden textarea from receiving focus (suppressing the soft keyboard). That was fine before Tailscale-exposed deployments, but users accessing `agent-deck web` from a phone could only view, not type. All mobile input restrictions were client-side; the server already accepts `input` WS messages and gates them on `--read-only`. This commit removes the mobile gate in `TerminalPanel.js` so: - `disableStdin` is now driven solely by the server-sent `readOnly`. - `terminal.onData` is registered unconditionally. - The mobile `touchstart` preventDefault is deleted, letting xterm's hidden textarea focus on tap and pop the soft keyboard. - The yellow "READ-ONLY: terminal input is disabled on mobile" banner is removed; the `--read-only` flag remains the sole source of truth for read-only deployments. Touch-drag scrollback continues to work: `installTouchScroll` never called preventDefault on touchstart, only on touchmove. The removed listener was the mobile-only suppressor, not the scroll handler. Battery optimizations unrelated to input are kept unchanged: - `cursorBlink: !mobile` - WebGL preload is skipped on mobile - `terminal.focus()` on connect stays desktop-only so opening a session on mobile doesn't auto-pop the keyboard. Test updates (TDD-first): - `tests/e2e/visual/p1-bug6-terminal-padding.spec.ts`: flip the banner assertion to pin absence; add two new structural tests asserting `terminal.onData` is not gated on `!mobile` and `disableStdin` is not OR-ed with `mobile`. - `tests/e2e/visual/p8-perf-e-listener-cleanup.spec.ts`: drop expected `controller.signal` count from 9 to 8 (the mobile-only touchstart listener is gone) and update doc comments. Verification: - `go test ./internal/web/... -race -count=1` passes (TestTmuxPTYBridgeResize is pre-existing flaky on HEAD, same rate with and without this change). - Updated/new Playwright structural specs pass (5/5). --- internal/web/static/app/TerminalPanel.js | 42 +++++-------------- .../visual/p1-bug6-terminal-padding.spec.ts | 24 ++++++++++- .../visual/p8-perf-e-listener-cleanup.spec.ts | 15 +++---- 3 files changed, 41 insertions(+), 40 deletions(-) diff --git a/internal/web/static/app/TerminalPanel.js b/internal/web/static/app/TerminalPanel.js index 9df51d8b..36ef9368 100644 --- a/internal/web/static/app/TerminalPanel.js +++ b/internal/web/static/app/TerminalPanel.js @@ -70,7 +70,6 @@ export function TerminalPanel() { const containerRef = useRef(null) const ctxRef = useRef(null) // { terminal, fitAddon, ws, resizeObserver, controller, decoder, reconnectTimer, reconnectAttempt, wsReconnectEnabled, terminalAttached } const sessionId = selectedIdSignal.value - const isMobile = isMobileDevice() // Signal vanilla app.js to suppress its terminal path while TerminalPanel is mounted useEffect(() => { @@ -80,8 +79,8 @@ export function TerminalPanel() { // Cleanup function: dispose terminal, close WS, remove observers. // PERF-E: a single controller.abort() detaches every event listener - // registered inside the main useEffect (9 total: 4 touch, 1 window - // resize, 1 anonymous mobile touchstart, 4 ws). + // registered inside the main useEffect (8 total: 4 touch, 1 window + // resize, 4 ws). const cleanup = useCallback(() => { const ctx = ctxRef.current if (!ctx) return @@ -112,7 +111,7 @@ export function TerminalPanel() { const terminal = new Terminal({ convertEol: false, cursorBlink: !mobile, - disableStdin: mobile, + disableStdin: false, fontFamily: 'IBM Plex Mono, Menlo, Consolas, monospace', fontSize: 13, scrollback: 10000, @@ -151,7 +150,7 @@ export function TerminalPanel() { fitAddon.fit() // PERF-E: single AbortController for every listener registered in this - // effect. Calling controller.abort() in the cleanup detaches all 9 + // effect. Calling controller.abort() in the cleanup detaches all 8 // listeners in one call -- replaces the previously incomplete manual // cleanup that only removed touchstart. const controller = new AbortController() @@ -226,22 +225,11 @@ export function TerminalPanel() { // AbortController (PERF-E). No local dispose handle is needed. installTouchScroll(container, terminal.element, controller) - // Keyboard input forwarding (desktop only) - let inputDisposable = null - if (!mobile) { - inputDisposable = terminal.onData((data) => { - if (!ctx.ws || ctx.ws.readyState !== WebSocket.OPEN || !ctx.terminalAttached || readOnlySignal.value) return - ctx.ws.send(JSON.stringify({ type: 'input', data })) - }) - } - - // Prevent mobile soft keyboard by blocking touch-focus on the hidden textarea - if (mobile) { - container.addEventListener('touchstart', (e) => { e.preventDefault() }, { - passive: false, - signal: controller.signal, - }) - } + // Keyboard input forwarding (desktop + mobile; server gates on ReadOnly). + const inputDisposable = terminal.onData((data) => { + if (!ctx.ws || ctx.ws.readyState !== WebSocket.OPEN || !ctx.terminalAttached || readOnlySignal.value) return + ctx.ws.send(JSON.stringify({ type: 'input', data })) + }) terminal.writeln('Connecting to terminal...') @@ -292,7 +280,7 @@ export function TerminalPanel() { if (payload.type === 'status') { if (payload.event === 'connected') { readOnlySignal.value = !!payload.readOnly - if (terminal) terminal.options.disableStdin = !!payload.readOnly || mobile + if (terminal) terminal.options.disableStdin = !!payload.readOnly wsStateSignal.value = 'connected' } else if (payload.event === 'terminal_attached') { ctx.terminalAttached = true @@ -340,7 +328,7 @@ export function TerminalPanel() { // Cleanup on unmount or sessionId change return () => { - if (inputDisposable) inputDisposable.dispose() + inputDisposable.dispose() clearTimeout(resizeTimer) cleanup() } @@ -352,14 +340,6 @@ export function TerminalPanel() { return html`
- ${isMobile && html` -
- READ-ONLY: terminal input is disabled on mobile -
- `}
diff --git a/tests/e2e/visual/p1-bug6-terminal-padding.spec.ts b/tests/e2e/visual/p1-bug6-terminal-padding.spec.ts index 5b34bdc9..453ccc6b 100644 --- a/tests/e2e/visual/p1-bug6-terminal-padding.spec.ts +++ b/tests/e2e/visual/p1-bug6-terminal-padding.spec.ts @@ -77,14 +77,34 @@ test.describe('BUG #6 / LAYT-03 — terminal panel has 16px padding on all edges ).toBe(true); }); - test('structural: mobile READ-ONLY banner is untouched', () => { + test('structural: mobile READ-ONLY banner is absent (mobile input enabled)', () => { const src = readTerminalPanelSrc(); expect( /READ-ONLY: terminal input is disabled on mobile/.test(src), - 'TerminalPanel.js mobile READ-ONLY banner was removed — LAYT-03 must leave the mobile banner above the padded wrapper intact.', + 'TerminalPanel.js must NOT render the legacy mobile READ-ONLY banner; mobile input is enabled and only the server --read-only flag disables input now.', + ).toBe(false); + }); + + test('structural: terminal.onData is not gated on !mobile', () => { + const src = readTerminalPanelSrc(); + expect( + /if \(!mobile\)\s*\{\s*inputDisposable\s*=\s*terminal\.onData/.test(src), + 'TerminalPanel.js must not gate terminal.onData behind !mobile; mobile input is enabled.', + ).toBe(false); + expect( + /terminal\.onData\s*\(/.test(src), + 'TerminalPanel.js must retain an unconditional terminal.onData(...) call to forward keystrokes to the tmux bridge.', ).toBe(true); }); + test('structural: disableStdin is not OR-ed with mobile on status messages', () => { + const src = readTerminalPanelSrc(); + expect( + /disableStdin\s*=\s*!!payload\.readOnly\s*\|\|\s*mobile/.test(src), + 'TerminalPanel.js must not OR mobile into disableStdin; only payload.readOnly should disable input.', + ).toBe(false); + }); + // RUNTIME — skips without a fixture session; measures real computed styles. test('runtime: xterm wrapper has 16px padding on all four edges', async ({ page }) => { await page.setViewportSize({ width: 1280, height: 800 }); diff --git a/tests/e2e/visual/p8-perf-e-listener-cleanup.spec.ts b/tests/e2e/visual/p8-perf-e-listener-cleanup.spec.ts index 4d9e5a1c..da899b4f 100644 --- a/tests/e2e/visual/p8-perf-e-listener-cleanup.spec.ts +++ b/tests/e2e/visual/p8-perf-e-listener-cleanup.spec.ts @@ -6,14 +6,15 @@ import { join } from 'path'; * Phase 8 / Plan 02 / Task 1: PERF-E regression test (TerminalPanel listener leak). * * CURRENT STATE (verified by grep of TerminalPanel.js as of 2026-04-09): - * - 9 addEventListener sites inside the main useEffect: + * - 8 addEventListener sites inside the main useEffect: * - 4 container touch listeners (touchstart, touchmove, touchend, touchcancel) * - 1 window resize listener (already uses a LOCAL windowResizeController -- partially done) - * - 1 anonymous touchstart preventDefault on container (mobile only) * - 4 ws.addEventListener (open, message, error, close) + * - The mobile-only anonymous touchstart preventDefault was removed when + * mobile console input was enabled; that dropped the site count from 9 to 8. * - Only 1 of these currently uses controller.signal (the window resize block). * - The existing cleanup at line 67 only manually removes the first touchstart; the - * remaining 8 listeners leak on every unmount / reconnect. + * remaining 7 listeners leak on every unmount / reconnect. * * FIX TO ENFORCE (PERF-E): * - ONE new AbortController() declared at the top of the useEffect. @@ -53,13 +54,13 @@ test.describe('PERF-E -- TerminalPanel listener cleanup via AbortController', () ).toBe(true); }); - test('structural: contains controller.signal at least 9 times (one per addEventListener site)', () => { + test('structural: contains controller.signal at least 8 times (one per addEventListener site)', () => { const src = source(); const matches = src.match(/controller\.signal/g) || []; expect( matches.length, - `Expected controller.signal to appear on every addEventListener site (>=9), found ${matches.length}. Sites: 4 touch on container + 1 window resize + 1 anonymous touchstart + 4 ws.`, - ).toBeGreaterThanOrEqual(9); + `Expected controller.signal to appear on every addEventListener site (>=8), found ${matches.length}. Sites: 4 touch on container + 1 window resize + 4 ws. (Mobile-only touchstart preventDefault was removed when mobile input was enabled.)`, + ).toBeGreaterThanOrEqual(8); }); test('structural: contains controller.abort() in the effect cleanup', () => { @@ -91,7 +92,7 @@ test.describe('PERF-E -- TerminalPanel listener cleanup via AbortController', () ).toEqual([]); }); - test('structural: no manual removeEventListener for the 9 migrated listeners', () => { + test('structural: no manual removeEventListener for the 8 migrated listeners', () => { const src = source(); // The AbortController pattern replaces manual removeEventListener. // If any removeEventListener for touch* events remains, the cleanup From ab89add29e1900cb23a7b96e282b8a94c9539f04 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 19 Apr 2026 20:36:49 +0000 Subject: [PATCH 2/2] docs(changelog): note mobile read-only behavior change Operators who relied on the implicit client-side mobile read-only default need to switch to `agent-deck web --read-only` to preserve that behavior. Flag this under an Unreleased / Changed entry so release notes pick it up. --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 098b43dd..3f431e27 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ All notable changes to Agent Deck will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Changed +- **Mobile web terminal input**: mobile clients (`pointer: coarse`) no longer enforce an implicit read-only mode in the web UI. Keystrokes from phones/tablets now flow to the tmux session like any other client. To preserve the previous behavior, start the web server with `agent-deck web --read-only` — the server-side flag now owns read-only enforcement for all devices. + ## [1.5.4] - 2026-04-16 ### Added