diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 76d8b51e..45d02ca5 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -1068,6 +1068,8 @@ Tests: `test/mobile-prompt-composer.test.ts` (in the CI gate, deliberately not u ⚠️ **A scroll ends in `touchend` too** — dismissing there closes the keyboard and drops the composer mid-read, so travel is tracked from `touchstart` and multi-touch is never a tap. Both classifiers MUST share one threshold: `initTerminal`'s `TAP_THRESHOLD` reads `MOBILE_KEYBOARD_DISMISS_TAP_SLOP`, since a gesture the terminal calls a scroll and the dismiss handler calls a tap is exactly that bug. +⚠️ **An accessory key that keeps the keyboard up must not take focus on the press.** The keys that act on the terminal and leave the keyboard open (`ACCESSORY_REFOCUS_ACTIONS` in keyboard-accessory.js: scroll, arrows, Tab, Shift+Tab, Ctrl, Esc, clear input and the rest) cancel the `mousedown` default, the same pattern as the Attach input, so the hidden textarea keeps focus through the press; the `terminal.focus()` in the click handler stays as the fallback. Letting the tap blur the terminal and refocusing it afterwards made every such key start a keyboard dismissal, a blur/focus pair per tap that shifted the toolbar each time. A first tap on clear/compact only arms the confirm and is treated the same; the confirming tap, and keys that open a dialog or dismiss the keyboard on purpose, keep the default. Pinned against the real module by `test/accessory-keeps-terminal-focus.test.ts`. + ⚠️ **The gate excludes `test/mobile/**`, so CI cannot see the only test covering (1)** — run `npm run test:mobile -- test/mobile/keyboard.test.ts` by hand and diff the FAIL list against master. (Not `npm test --`: the gate's config excludes that path, so a file filter pointing into it matches nothing and exits green having run zero tests.) That blind spot is why merging the two PRs, which conflicted semantically but not textually, produced a red suite with two green CI checks. ### SSE staleness watchdog diff --git a/src/web/CLAUDE.md b/src/web/CLAUDE.md index 30ef4952..328d0b73 100644 --- a/src/web/CLAUDE.md +++ b/src/web/CLAUDE.md @@ -76,7 +76,7 @@ Frontend JS modules have `@fileoverview` with `@dependency`/`@loadorder` tags. L **WebSocket output-gap reconcile** (`_wsOutputGapSession`, app.js): output frames carry no sequence number, so an unintentional WS close while SSE stays up marks the session and the next open reconciles. ⚠️ The marker is cleared only once a repaint actually happened (`_markTerminalBufferReconciled()`, never in a `finally`). → [architecture-invariants#websocket-output-gap-reconcile](../../docs/architecture-invariants.md#websocket-output-gap-reconcile) -**Dismissing the on-screen keyboard** (`terminal-ui.js`): two gestures blur the terminal's hidden textarea. (1) `_installMobileKeyboardDismiss()`, a document `touchend` that must never fire inside `#terminalContainer` or on a control (`MOBILE_KEYBOARD_DISMISS_EXEMPT_SELECTOR`, via `closest()`; roving-tabindex items sit at `tabindex=-1`, so the grouped rail's are listed as `[role="treeitem"]`). (2) In `_handleMobileTerminalTap`, a second tap on inert `content` blurs; the prompt row keeps focus-then-position. ⚠️ A scroll also ends in `touchend`: both classifiers must share one threshold (`TAP_THRESHOLD` reads `MOBILE_KEYBOARD_DISMISS_TAP_SLOP`), and multi-touch is never a tap. ⚠️ CI cannot see the only test for (1): run `npm run test:mobile -- test/mobile/keyboard.test.ts` by hand and diff the FAIL list against master. → [architecture-invariants#dismissing-the-on-screen-keyboard](../../docs/architecture-invariants.md#dismissing-the-on-screen-keyboard) +**Dismissing the on-screen keyboard** (`terminal-ui.js`): two gestures blur the terminal's hidden textarea. (1) `_installMobileKeyboardDismiss()`, a document `touchend` that must never fire inside `#terminalContainer` or on a control (`MOBILE_KEYBOARD_DISMISS_EXEMPT_SELECTOR`, via `closest()`; roving-tabindex items sit at `tabindex=-1`, so the grouped rail's are listed as `[role="treeitem"]`). (2) In `_handleMobileTerminalTap`, a second tap on inert `content` blurs; the prompt row keeps focus-then-position. ⚠️ A scroll also ends in `touchend`: both classifiers must share one threshold (`TAP_THRESHOLD` reads `MOBILE_KEYBOARD_DISMISS_TAP_SLOP`), and multi-touch is never a tap. ⚠️ An accessory key that keeps the keyboard up (`ACCESSORY_REFOCUS_ACTIONS`, keyboard-accessory.js) cancels its `mousedown` default, so the press does not blur the terminal; the click-time `terminal.focus()` stays as the fallback. Letting the tap blur and refocusing afterwards made every such key start a dismissal, a blur/focus pair per tap. A first tap on clear/compact only arms the confirm and is treated the same (`test/accessory-keeps-terminal-focus.test.ts`). ⚠️ CI cannot see the only test for (1): run `npm run test:mobile -- test/mobile/keyboard.test.ts` by hand and diff the FAIL list against master. → [architecture-invariants#dismissing-the-on-screen-keyboard](../../docs/architecture-invariants.md#dismissing-the-on-screen-keyboard) **Phone toolbar: Enter replaces Shell** (post-1.8.0): inside `@media (max-width: 599px)` `btn-shell` is `display:none` and `btn-enter` takes its slot (`order: 4`); starting a shell moved into the Run dropdown (`Terminal / Shell` → `setRunMode('shell')` → `run()` → `runShell()`, button label "Run SH"). `runMode` is `z.string().max(20)` server-side, so new modes need no schema change. Desktop and tablet keep the green Run Shell button unchanged. diff --git a/src/web/public/keyboard-accessory.js b/src/web/public/keyboard-accessory.js index d76aa9c1..33391135 100644 --- a/src/web/public/keyboard-accessory.js +++ b/src/web/public/keyboard-accessory.js @@ -647,6 +647,9 @@ const COMPOSER_INPUT_FRAME_LIMIT = 64 * 1024; const COMPOSER_PASTE_START = '\x1b[200~'; const COMPOSER_PASTE_END = '\x1b[201~'; +// Accessory keys that act on the terminal and leave the keyboard up. +const ACCESSORY_REFOCUS_ACTIONS = new Set(['scroll-up', 'scroll-down', 'arrow-left', 'arrow-right', 'tab', 'shift-tab', 'shift-left', 'shift-right', 'ctrl', 'ctrl-o', 'opt-enter', 'esc', 'effort-max', 'clear-input']); + /** * KeyboardAccessoryBar - Quick action buttons shown above keyboard when typing. */ @@ -802,6 +805,21 @@ const KeyboardAccessoryBar = { this.syncCodexKeys(); this._syncComposerDraftIndicator(); + // A key that keeps the keyboard up must not take focus on press at all. + // Letting the tap blur the terminal and refocusing it in the click handler + // (kept below as the fallback) made every such key start a keyboard + // dismissal: a blur/focus pair per tap. Same pattern as the Attach input + // (image-input.js). A first tap on clear/compact only arms the confirm. + this.element.addEventListener('mousedown', (e) => { + const btn = e.target.closest('.accessory-btn'); + if (!btn) return; + const action = btn.dataset.action; + if (ACCESSORY_REFOCUS_ACTIONS.has(action) || + ((action === 'clear' || action === 'compact') && this._confirmAction !== action)) { + e.preventDefault(); + } + }); + // Add click handlers — preventDefault stops event from reaching terminal this.element.addEventListener('click', (e) => { const btn = e.target.closest('.accessory-btn'); @@ -813,8 +831,7 @@ const KeyboardAccessoryBar = { this.handleAction(action, btn); // Refocus terminal so keyboard stays open (tap blurs terminal → keyboard dismisses → toolbar shifts) - const refocusActions = new Set(['scroll-up', 'scroll-down', 'arrow-left', 'arrow-right', 'tab', 'shift-tab', 'shift-left', 'shift-right', 'ctrl', 'ctrl-o', 'opt-enter', 'esc', 'effort-max', 'clear-input']); - if (refocusActions.has(action) || + if (ACCESSORY_REFOCUS_ACTIONS.has(action) || ((action === 'clear' || action === 'compact') && this._confirmAction)) { if (typeof app !== 'undefined' && app.terminal) { app.terminal.focus(); diff --git a/test/accessory-keeps-terminal-focus.test.ts b/test/accessory-keeps-terminal-focus.test.ts new file mode 100644 index 00000000..a9e979ee --- /dev/null +++ b/test/accessory-keeps-terminal-focus.test.ts @@ -0,0 +1,105 @@ +/** + * @fileoverview Accessory keys that keep the keyboard up must not take focus on + * press. A tap used to blur the terminal's textarea and the click handler then + * refocused it, so every arrow/Tab/Esc tap started a keyboard dismissal. The bar + * now cancels the mousedown default for exactly those keys (and the first, + * arming tap of clear/compact); every other key keeps the old behavior. + * + * Loaded with `vm` against a minimal fake DOM, like the other accessory-bar tests. + */ +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import vm from 'node:vm'; +import { describe, expect, it, vi } from 'vitest'; + +type Listener = (e: unknown) => void; +type Bar = { + init(): void; + _confirmAction: string | null; + syncReadMyMind(): void; + syncCodexKeys(): void; + _syncComposerDraftIndicator(): void; +}; + +function loadBar() { + const listeners: Record = {}; + const element = { + className: '', + innerHTML: '', + addEventListener: (type: string, fn: Listener) => { + listeners[type] = fn; + }, + }; + const context = vm.createContext({ + console, + document: { createElement: () => element, querySelector: () => null }, + MobileDetection: { isTouchDevice: () => true }, + localStorage: { getItem: () => null, setItem: () => {} }, + }); + vm.runInContext( + `${readFileSync(resolve('src/web/public/keyboard-accessory.js'), 'utf8')}\nglobalThis.__bar = KeyboardAccessoryBar;`, + context + ); + const bar = (context as unknown as { __bar: Bar }).__bar; + bar.syncReadMyMind = () => {}; + bar.syncCodexKeys = () => {}; + bar._syncComposerDraftIndicator = () => {}; + bar.init(); + // `null` is a press on the bar itself (padding, a gap): closest() finds no key. + const press = (action: string | null) => { + const preventDefault = vi.fn(); + listeners.mousedown({ + target: { closest: () => (action === null ? null : { dataset: { action } }) }, + preventDefault, + }); + return preventDefault.mock.calls.length > 0; + }; + return { bar, press }; +} + +describe('accessory keys keep the terminal focused on press', () => { + it('cancels the press default for keys that leave the keyboard up', () => { + const { press } = loadBar(); + for (const action of [ + 'scroll-up', + 'scroll-down', + 'arrow-left', + 'arrow-right', + 'tab', + 'shift-tab', + 'shift-left', + 'shift-right', + 'ctrl', + 'ctrl-o', + 'opt-enter', + 'esc', + 'effort-max', + 'clear-input', + ]) { + expect(press(action), action).toBe(true); + } + }); + + it('leaves keys that move focus or dismiss the keyboard alone', () => { + const { press } = loadBar(); + for (const action of ['compose', 'readmymind', 'pick-path', 'paste', 'dismiss', 'init']) { + expect(press(action), action).toBe(false); + } + }); + + it('cancels only the arming tap of clear/compact, not the confirming one', () => { + const { bar, press } = loadBar(); + bar._confirmAction = null; + expect(press('clear')).toBe(true); + bar._confirmAction = 'clear'; + expect(press('clear')).toBe(false); + expect(press('compact')).toBe(true); + bar._confirmAction = 'compact'; + expect(press('compact')).toBe(false); + }); + + it('ignores a press outside a key', () => { + const { press } = loadBar(); + expect(press(null)).toBe(false); + }); +});