fix(mobile): keep the one-shot Ctrl armed through terminal-generated reports

Review of #268 turned up two defects, both verified against a real shell
session on an isolated instance.

1. A tap spent the modifier. The onData hook consumed every chunk while
   armed, but not every chunk is a keystroke: a shell session keeps the
   narrow scrollback strip, so mouse DECSETs reach the browser, and with
   vim/htop running a tap arrives as `\x1b[<0;31;23M`. Measured in the
   real app: armed, one tap, disarmed, and the Ctrl button read as dead.
   The hook now skips mouse and focus reports via a new
   `CodemanTerminalInput.isTerminalFocusOrMouseReport()`; they still reach
   the PTY, they just no longer stand in for the next key. Focus reports
   are covered for the same reason even though FOCUS_ESCAPE_FILTER in
   session.ts strips DECSET 1004 today, since the bar refocuses the
   terminal after every key and would spend the modifier on its own
   `\x1b[I` the moment that filter changed.

2. The armed style did not land on the four light skins. The competing
   rule is (0,3,1), not (0,2,1) as the comments claimed: `:is()` takes the
   specificity of its most specific argument and that list holds
   `.btn-toolbar.btn-shell`, so it outranked the (0,3,0) armed rules in
   both stylesheets. Measured across all seven skins at 390px, armed and
   resting backgrounds were byte-identical on paper-gray, solarized-light,
   catppuccin-latte and rose-pine-dawn. The light-skin rule now excludes
   the state as `.accessory-btn:not(.armed)`, which fixes phone and tablet
   at once; adding another class to the armed rules would only have moved
   the tie.

Tests: 20 more cases in test/mobile-shell-keyboard.test.ts (the report
classifier, the gate's effect on the modifier, and a static guard on the
light-skin selector, since the existing E2E background assertion passes on
a light skin and the browser suite runs the dark default), plus a browser
regression that taps the terminal with mouse reporting on.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer
2026-08-10 04:07:19 +02:00
parent 9b9f2c21e9
commit 7c2a49d432
6 changed files with 229 additions and 9 deletions
+1 -1
View File
@@ -270,7 +270,7 @@ Frontend JS modules have `@fileoverview` with `@dependency`/`@loadorder` tags. L
**WebGL renderer toggle** (`webglRendererEnabled`, per-device): the GPU-stall watchdog's sticky `codeman-webgl-disabled` marker survives page loads and is cleared only by an explicit OFF→ON save or `?webgl=force`. `?nowebgl` forces the DOM renderer per-load. → [architecture-invariants#webgl-renderer-toggle](docs/architecture-invariants.md#webgl-renderer-toggle)
**Shell keyboard accessory bar + one-shot Ctrl** (issue #262, `keyboard-accessory.js`): a **shell**-mode session automatically swaps the mobile accessory bar for terminal controls (Ctrl, Esc, Tab, four arrows, paste, dismiss); every other mode keeps the agent bar. `setMode()` now records the user's `extendedKeyboardBar` preference as the **base** layout and `refreshForActiveSession()` (called from `selectSession`) resolves base-vs-shell, so a settings save during a shell session cannot yank the bar away and switching back restores the user's choice. ⚠️ **Ctrl is a ONE-SHOT modifier applied in `terminal.onData`, not in a keydown handler**: a virtual keyboard emits no usable key events, so the character only exists as onData text. The hook sits AFTER `shouldSuppressTerminalQueryResponse` (xterm answers DA/CPR through onData too, and one of those would silently spend the modifier) and BEFORE every send path, so the control byte follows the normal control-char route. ⚠️ It must disarm on ALL of: use, second tap, any other accessory key, session switch, keyboard dismissal, and a layout swap; a modifier left armed turns the next innocent keystroke into a control byte. Mapping is `ctrlByteFor()` (`code & 0x1f` over @A-Z[\]^_ and a-z, plus Ctrl+Space=NUL / Ctrl+?=DEL); characters with no control equivalent pass through unchanged, like a hardware keyboard. The armed style needs **three classes** (`.accessory-btn.accessory-btn-ctrl.armed`) to outrank mobile.css's light-skin `.accessory-btn` rule at (0,2,1).
**Shell keyboard accessory bar + one-shot Ctrl** (issue #262, `keyboard-accessory.js`): a **shell**-mode session automatically swaps the mobile accessory bar for terminal controls (Ctrl, Esc, Tab, four arrows, paste, dismiss); every other mode keeps the agent bar. `setMode()` now records the user's `extendedKeyboardBar` preference as the **base** layout and `refreshForActiveSession()` (called from `selectSession`) resolves base-vs-shell, so a settings save during a shell session cannot yank the bar away and switching back restores the user's choice. ⚠️ **Ctrl is a ONE-SHOT modifier applied in `terminal.onData`, not in a keydown handler**: a virtual keyboard emits no usable key events, so the character only exists as onData text. The hook sits AFTER `shouldSuppressTerminalQueryResponse` (xterm answers DA/CPR through onData too, and one of those would silently spend the modifier) and BEFORE every send path, so the control byte follows the normal control-char route. ⚠️ **Not every onData chunk is a keystroke**, and the query filter is not enough on its own: xterm ALSO emits mouse and focus reports on its own initiative, so the hook skips them via `isTerminalFocusOrMouseReport()` (they still reach the PTY, they just don't count as the next key). The mouse half is live — a shell session keeps the NARROW strip, so mouse DECSETs reach the browser and one tap while vim/htop runs spent the armed modifier silently (measured). The focus half is defense in depth: `FOCUS_ESCAPE_FILTER` in `session.ts` strips `\x1b[?1004h` from every PTY read, so `sendFocusMode` never turns on today; if it ever did, the bar's own post-key refocus would emit `\x1b[I` and eat the modifier before the user typed. ⚠️ It must disarm on ALL of: use, second tap, any other accessory key, session switch, keyboard dismissal, and a layout swap; a modifier left armed turns the next innocent keystroke into a control byte. Mapping is `ctrlByteFor()` (`code & 0x1f` over @A-Z[\]^_ and a-z, plus Ctrl+Space=NUL / Ctrl+?=DEL); characters with no control equivalent pass through unchanged, like a hardware keyboard. ⚠️ The armed style is `.accessory-btn.accessory-btn-ctrl.armed` (0,3,0) in BOTH stylesheets, and it cannot outrank mobile.css's light-skin repaint at **(0,3,1)** (`:is()` inherits its most specific argument, and that list holds `.btn-toolbar.btn-shell`) — so that rule excludes the state by hand as `.accessory-btn:not(.armed)`. Without the exclusion the armed button renders identically to a resting one on all four light skins, which is worse than no armed style at all.
**Phone toolbar: Enter replaces Shell** (post-1.8.0): inside `@media (max-width: 430px)` `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.
+10 -3
View File
@@ -1163,8 +1163,9 @@ html.mobile-init .file-browser-panel {
}
/* Armed one-shot Ctrl (shell bar, issue #262). Phone palette is hardcoded in
this block, so the state needs its own entry here; three classes to outrank
the light-skin .accessory-btn rule at the bottom of this file. */
this block, so the state needs its own entry here. Three classes beat the
plain .accessory-btn rules; the light-skin rule at the bottom of this file
is higher still at (0,3,1) and is excluded by hand there, not outranked. */
.accessory-btn.accessory-btn-ctrl.armed {
background: #2563eb;
border-color: rgba(59, 130, 246, 0.9);
@@ -2918,7 +2919,13 @@ html:is([data-skin="paper-gray"], [data-skin="solarized-light"], [data-skin="cat
color: var(--text);
}
html:is([data-skin="paper-gray"], [data-skin="solarized-light"], [data-skin="catppuccin-latte"], [data-skin="rose-pine-dawn"]) :is(.btn-voice-mobile, .btn-settings-mobile, .btn-toolbar.btn-shell, .toolbar .btn-case-add, .accessory-btn) {
/* `.accessory-btn:not(.armed)` on purpose: this selector is (0,3,1) — `:is()`
takes the specificity of its most specific argument, and `.btn-toolbar
.btn-shell` is two classes — so it OUTRANKS the (0,3,0) armed-Ctrl rules in
both stylesheets and repainted the armed modifier back to a resting button on
all four light skins. Excluding the state here fixes phone and tablet at once;
adding a class to the armed rules would only have moved the tie. */
html:is([data-skin="paper-gray"], [data-skin="solarized-light"], [data-skin="catppuccin-latte"], [data-skin="rose-pine-dawn"]) :is(.btn-voice-mobile, .btn-settings-mobile, .btn-toolbar.btn-shell, .toolbar .btn-case-add, .accessory-btn:not(.armed)) {
background: var(--control-bg);
border-color: var(--control-border);
color: var(--text-dim);
+6 -4
View File
@@ -12081,10 +12081,12 @@ body.touch-device.cjk-input-visible .main {
color: var(--yellow);
}
/* Armed one-shot Ctrl (shell bar, issue #262). Three classes on purpose: the
light-skin compatibility rule in mobile.css repaints every .accessory-btn at
specificity (0,2,1), so a two-class rule here would lose and the modifier
would look unarmed on the light skins. */
/* Armed one-shot Ctrl (shell bar, issue #262). Three classes on purpose, to beat
the plain .accessory-btn rules. It still cannot outrank the light-skin
compatibility rule in mobile.css, which repaints every .accessory-btn at
(0,3,1) — `:is()` inherits its most specific argument — so that rule excludes
`.armed` by hand. Without the exclusion the modifier looks unarmed on the four
light skins, which is worse than having no armed style at all. */
.accessory-btn.accessory-btn-ctrl.armed {
background: var(--accent);
border-color: var(--accent);
+45 -1
View File
@@ -15,6 +15,15 @@
(function (global) {
const TERMINAL_QUERY_RESPONSE_PATTERN = /^\x1b\[[\?>=]?[\d;]*[cnR]$/;
const TERMINAL_OSC_RESPONSE_PATTERN = /^\x1b\][\d;]*[^\x07\x1b]*(?:\x07|\x1b\\)$/;
// Pointer and focus reports xterm emits through onData on the terminal's OWN
// initiative, with no key pressed: SGR mouse (DECSET 1006, also 1016), legacy
// X10 mouse (DECSET 1000 — three raw bytes after CSI M) and focus in/out
// (DECSET 1004). They are not query REPLIES, so the query-response filter
// above does not match them, and they must keep reaching the PTY. What they
// must NOT do is stand in for a keystroke: see isTerminalFocusOrMouseReport.
const MOUSE_SGR_REPORT_PATTERN = /^\x1b\[<\d+;\d+;\d+[Mm]$/;
const MOUSE_X10_REPORT_PATTERN = /^\x1b\[M[\s\S]{3}$/;
const FOCUS_REPORT_PATTERN = /^\x1b\[[IO]$/;
// Grace window after a manual scroll-up gesture during which sticky-scroll is
// suppressed, so high-frequency Codex status redraws don't snap the viewport
// back to the bottom while the user is inspecting earlier output.
@@ -106,6 +115,30 @@
return isTerminalQueryResponse(data);
}
/**
* Did the terminal generate this chunk itself, rather than a human pressing a
* key? True for mouse and focus reports (issue #262).
*
* Consumers that treat one onData chunk as "the next keystroke" must skip
* these. The one-shot Ctrl modifier is why this exists, and the MOUSE half is
* the live one: a shell session keeps the narrow scrollback strip, so mouse
* DECSETs reach the browser and anything the user runs that enables tracking
* (vim, htop, less) turns a tap into `\x1b[<0;31;23M`. Measured in a real
* shell session: with Ctrl armed, one tap on the terminal spent it silently.
*
* Focus reports are the same class and cost nothing to cover, but they cannot
* reach xterm today: `FOCUS_ESCAPE_FILTER` in session.ts strips `\x1b[?1004h`
* (and the reports themselves) from every PTY read, so `sendFocusMode` never
* turns on. Were that filter to go, the Ctrl button would spend the modifier
* on its OWN refocus — the bar refocuses the terminal after every key so the
* keyboard stays open, and that refocus emits `\x1b[I`.
*/
function isTerminalFocusOrMouseReport(data) {
return (
FOCUS_REPORT_PATTERN.test(data) || MOUSE_SGR_REPORT_PATTERN.test(data) || MOUSE_X10_REPORT_PATTERN.test(data)
);
}
// Per-skin xterm.js palettes. The 'daylight-blue' object equals the legacy hardcoded
// theme, so default behavior is unchanged. Shared at module scope and exported on the
// global so both terminal-ui.js (main terminal) and panels-ui.js (teammate terminals,
@@ -134,6 +167,7 @@
global.CodemanTerminalInput = {
isTerminalQueryResponse,
shouldSuppressTerminalQueryResponse,
isTerminalFocusOrMouseReport,
isComposerNavKey,
classifyPredictInput,
isCodexComposerRow,
@@ -938,7 +972,17 @@ Object.assign(CodemanApp.prototype, {
// own DA/CPR replies can never spend the modifier, and BEFORE every
// send path so the control byte follows the normal control-char route
// (immediate flush, local-echo state cleared).
if (typeof KeyboardAccessoryBar !== 'undefined' && KeyboardAccessoryBar.isCtrlArmed?.()) {
//
// Mouse and focus reports are skipped rather than suppressed: they are
// real bytes the PTY still needs, they just were not typed by anyone.
// A shell session passes mouse DECSETs through, so with vim or htop
// running, one tap on the terminal used to spend the modifier silently
// (measured against a real shell). See isTerminalFocusOrMouseReport.
if (
typeof KeyboardAccessoryBar !== 'undefined' &&
KeyboardAccessoryBar.isCtrlArmed?.() &&
!window.CodemanTerminalInput?.isTerminalFocusOrMouseReport(data)
) {
data = KeyboardAccessoryBar.consumeCtrl(data);
}
+143
View File
@@ -17,6 +17,39 @@ import { describe, expect, it, vi } from 'vitest';
const keyboardSource = readFileSync(resolve('src/web/public/keyboard-accessory.js'), 'utf8');
const terminalSource = readFileSync(resolve('src/web/public/terminal-ui.js'), 'utf8');
type TerminalInput = { isTerminalFocusOrMouseReport(data: string): boolean };
let terminalInput: TerminalInput | null = null;
/**
* `CodemanTerminalInput` out of terminal-ui.js. Its IIFE only needs a window to
* hang the export on, but the rest of the file assigns to CodemanApp.prototype
* at top level, so constants.js + app.js load first — the same recipe as
* test/local-echo-codex-gating.test.ts.
*/
function loadTerminalInput(): TerminalInput {
if (terminalInput) return terminalInput;
const read = (file: string) => readFileSync(resolve(`src/web/public/${file}`), 'utf8');
const windowStub: Record<string, unknown> = { addEventListener: vi.fn(), removeEventListener: vi.fn() };
const context = vm.createContext({
console,
setInterval: vi.fn(),
clearInterval: vi.fn(),
setTimeout,
clearTimeout,
requestAnimationFrame: vi.fn(),
HTMLCanvasElement: class HTMLCanvasElement {},
WebSocket: { OPEN: 1 },
fetch: vi.fn(),
document: { addEventListener: vi.fn(), documentElement: { dataset: {} } },
localStorage: { length: 0, key: vi.fn(), getItem: vi.fn(), setItem: vi.fn(), removeItem: vi.fn() },
window: windowStub,
MobileDetection: { isTouchDevice: () => true, isHandheldDevice: () => false, getDeviceType: () => 'desktop' },
});
vm.runInContext(`${read('constants.js')}\n${read('app.js')}\n${terminalSource}`, context);
terminalInput = (windowStub as { CodemanTerminalInput?: TerminalInput }).CodemanTerminalInput!;
return terminalInput;
}
type FakeButton = {
dataset: { action: string };
classList: { has: Set<string>; toggle(name: string, on: boolean): void; contains(name: string): boolean };
@@ -341,4 +374,114 @@ describe('terminal input wiring', () => {
it('guards the hook so a page without the bar (desktop) still types normally', () => {
expect(terminalSource).toContain("typeof KeyboardAccessoryBar !== 'undefined'");
});
it('skips terminal-generated focus and mouse reports', () => {
// Pins the gate itself: without it the modifier is spent by the `\x1b[I`
// that the Ctrl button's own refocus emits (see the describe below).
expect(terminalSource).toContain('!window.CodemanTerminalInput?.isTerminalFocusOrMouseReport(data)');
});
});
describe('CodemanTerminalInput.isTerminalFocusOrMouseReport', () => {
const isReport = loadTerminalInput().isTerminalFocusOrMouseReport;
it.each([
['\x1b[I', 'focus in (DECSET 1004)'],
['\x1b[O', 'focus out (DECSET 1004)'],
['\x1b[<0;10;5M', 'SGR mouse press'],
['\x1b[<0;10;5m', 'SGR mouse release'],
['\x1b[<64;10;5M', 'SGR wheel up'],
['\x1b[M !!', 'legacy X10 mouse'],
])('classifies %j as terminal-generated (%s)', (data) => {
expect(isReport(data)).toBe(true);
});
it.each([
['c', 'a typed character'],
['\x03', 'a control byte'],
['\r', 'Enter'],
['\x1b', 'the Escape key'],
['\x1b[A', 'an arrow key'],
['\x1b[200~hi\x1b[201~', 'a bracketed paste'],
['\x1b[?1;2c', 'a DA reply'],
['I', 'the letter I'],
])('leaves %j alone (%s)', (data) => {
expect(isReport(data)).toBe(false);
});
});
describe('one-shot Ctrl vs terminal-generated reports', () => {
// The onData gate, as terminal-ui.js writes it. The wiring test above pins
// the real source; this proves the behavior the gate buys.
function feed(bar: Bar, data: string): string {
const isReport = loadTerminalInput().isTerminalFocusOrMouseReport;
return bar.isCtrlArmed() && !isReport(data) ? bar.consumeCtrl(data) : data;
}
function shellBar() {
const loaded = loadBar('shell');
loaded.bar.refreshForActiveSession();
return loaded;
}
it('survives a tap once an app in the pane turns mouse reporting on', () => {
const { bar } = shellBar();
// The live case: a shell session keeps the narrow scrollback strip, so mouse
// DECSETs reach the browser. Measured against a real shell with vim-style
// tracking on, one tap on the terminal spent the armed modifier silently.
bar.handleAction('ctrl');
expect(feed(bar, '\x1b[<0;10;5M')).toBe('\x1b[<0;10;5M');
expect(feed(bar, '\x1b[<0;10;5m')).toBe('\x1b[<0;10;5m');
expect(bar.isCtrlArmed()).toBe(true);
// ...so the character the user actually types is still the one modified.
expect(feed(bar, 'd')).toBe('\x04');
expect(bar.isCtrlArmed()).toBe(false);
});
it('survives a focus report, should one ever reach xterm', () => {
// Defense in depth: FOCUS_ESCAPE_FILTER (session.ts) strips `\x1b[?1004h`
// from every PTY read, so sendFocusMode never turns on today. If it did,
// the bar's own post-key refocus would emit `\x1b[I` and eat the modifier
// before the user typed a single character.
const { bar } = shellBar();
bar.handleAction('ctrl');
expect(feed(bar, '\x1b[I')).toBe('\x1b[I');
expect(bar.isCtrlArmed()).toBe(true);
expect(feed(bar, 'c')).toBe('\x03');
});
it('still spends the modifier on a paste, which is real input', () => {
const { bar } = shellBar();
bar.handleAction('ctrl');
expect(feed(bar, 'git status')).toBe('git status');
expect(bar.isCtrlArmed()).toBe(false);
});
});
describe('armed styling survives the light-skin overrides', () => {
const mobileCss = readFileSync(resolve('src/web/public/mobile.css'), 'utf8');
it('excludes .armed from the light-skin .accessory-btn repaint', () => {
// That selector is (0,3,1): `:is()` takes the specificity of its most
// specific argument and the list holds `.btn-toolbar.btn-shell`. It
// therefore OUTRANKS the (0,3,0) armed rules in both stylesheets, and a
// bare `.accessory-btn` there paints the armed modifier back to a resting
// button on all four light skins (measured across every skin at 390px).
const lightSkinRule = mobileCss
.split('\n')
.find((line) => line.includes('[data-skin="paper-gray"]') && line.includes('.btn-voice-mobile,'));
expect(lightSkinRule).toBeDefined();
expect(lightSkinRule).toContain('.accessory-btn:not(.armed)');
});
it('keeps an armed rule in both stylesheets', () => {
// mobile.css hardcodes the phone palette, styles.css carries the
// skin-aware one for everything wider.
expect(mobileCss).toContain('.accessory-btn.accessory-btn-ctrl.armed');
expect(readFileSync(resolve('src/web/public/styles.css'), 'utf8')).toContain(
'.accessory-btn.accessory-btn-ctrl.armed'
);
});
});
+24
View File
@@ -1146,6 +1146,30 @@ describe('Virtual Keyboard', () => {
expect(await typeAndCapture('d')).toEqual(['\x04']);
});
it('survives a terminal tap while the pane has mouse reporting on', async () => {
// A shell session keeps the narrow scrollback strip, so mouse DECSETs
// reach the browser: run vim or htop and xterm starts reporting taps
// through onData as \x1b[<0;31;23M. Those arrive on the same channel as
// typed characters, so a hook that treats every chunk as "the next
// keystroke" spends Ctrl on a tap and the button looks dead. Verified
// against a real shell session before this guard existed.
await activateSession('shell');
await page.evaluate(`app.terminal.write('\\x1b[?1000h\\x1b[?1006h')`);
await page.waitForTimeout(150);
await tapCtrl();
expect(await page.evaluate(`KeyboardAccessoryBar.isCtrlArmed()`)).toBe(true);
const box = await page.locator('.xterm-screen').first().boundingBox();
await page.touchscreen.tap(box!.x + box!.width / 2, box!.y + box!.height / 2);
await page.waitForTimeout(200);
expect(await page.evaluate(`KeyboardAccessoryBar.isCtrlArmed()`)).toBe(true);
expect(await typeAndCapture('c')).toEqual(['\x03']);
await page.evaluate(`app.terminal.write('\\x1b[?1000l\\x1b[?1006l')`);
});
it('cancels on a second tap of Ctrl', async () => {
await activateSession('shell');
await tapCtrl();