fix(mobile): apply the one-shot Ctrl on the CJK input path too

onData is not the only way keystrokes reach the PTY. With cjkInputEnabled
on, the CJK textarea owns the keyboard: onData returns early for
everything it swallows, and the focus router even redirects
terminal.focus() into the field, which is exactly where the accessory bar
sends focus after every key. So an armed modifier could neither fire NOR
be spent there — it survived until a session switch or keyboard dismissal
and then turned an innocent keystroke into a control byte, the failure
mode the whole disarm list exists to prevent.

`_handleCjkInput()` is that module's single choke point to the PTY, so
applying the modifier there covers typed characters, IME flushes, Enter,
backspace and arrows in one place, with the same policy as the onData
hook: the next single character is modified, anything longer merely
spends it. A committed CJK word therefore passes through untouched and
still clears the modifier.

Verified against a real shell session with the CJK field focused and
owning input (cjkActive true, focus in #cjkInput). Before: typing c left
a literal c in the pane, `sleep 300` kept running, and Ctrl stayed armed.
After: ^C in the pane, modifier disarmed, plain typing still literal.

Tests: 5 cases driving the real _handleCjkInput against the real bar,
both loaded into one vm scope (the bar is a const singleton, so a shared
script scope is what makes the bare reference resolve). Removing the fix
fails 3 of them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer
2026-08-10 04:25:31 +02:00
parent 7c2a49d432
commit c8ac04662d
3 changed files with 106 additions and 12 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. ⚠️ **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.
**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. ⚠️ **onData is not the only input path** — with `cjkInputEnabled` on, the CJK textarea owns the keyboard (onData returns early for everything it swallows, and the focus router sends `terminal.focus()` there, which is where the bar refocuses after every key), so `_handleCjkInput()` applies the modifier too. It is that module's single choke point to the PTY, so one call covers typed characters, IME flushes, Enter, backspace and arrows. Without it an armed modifier could neither fire NOR be spent, and survived to a later keystroke. 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.
+14
View File
@@ -2640,6 +2640,20 @@ Object.assign(CodemanApp.prototype, {
_crashDiag.log(`CJK send DROP no-session len=${text.length}`);
return;
}
// ── One-shot Ctrl (mobile shell bar, issue #262) ──
// While the CJK field is visible it OWNS the keyboard: onData returns early
// for everything it swallows, and the focus router even redirects
// terminal.focus() into it — which is where the accessory bar sends focus
// after every key. So the onData hook never sees these keystrokes, and an
// armed modifier could neither fire NOR be spent: it survived until a
// session switch and then turned an innocent keystroke into a control byte.
// This is the module's single choke point to the PTY, so applying it here
// covers typed characters, IME flushes, Enter, backspace and arrows at once.
// Same policy as the onData hook: the next single character is modified,
// anything longer merely spends the modifier.
if (typeof KeyboardAccessoryBar !== 'undefined' && KeyboardAccessoryBar.isCtrlArmed?.()) {
text = KeyboardAccessoryBar.consumeCtrl(text);
}
// Bypasses onData (like insertTerminalText): predictions cannot see this
if (this._localEchoPolicy === 'predict') this._predictiveEcho?.clearPredictions();
_crashDiag.log(`CJK send→${this.activeSessionId.slice(0, 8)} len=${text.length}`);
+91 -11
View File
@@ -18,16 +18,23 @@ const keyboardSource = readFileSync(resolve('src/web/public/keyboard-accessory.j
const terminalSource = readFileSync(resolve('src/web/public/terminal-ui.js'), 'utf8');
type TerminalInput = { isTerminalFocusOrMouseReport(data: string): boolean };
let terminalInput: TerminalInput | null = null;
type TerminalModule = {
terminalInput: TerminalInput;
CodemanApp: { prototype: Record<string, (...args: never[]) => unknown> };
bar: Bar;
};
let terminalModule: TerminalModule | 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.
* terminal-ui.js in a vm, with the REAL accessory bar in the same script scope
* (it is a `const` singleton, so only a shared scope makes the bare
* `KeyboardAccessoryBar` reference in the CJK path resolve). Its IIFE only
* needs a window to hang `CodemanTerminalInput` 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;
function loadTerminalModule(): TerminalModule {
if (terminalModule) return terminalModule;
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({
@@ -40,14 +47,28 @@ function loadTerminalInput(): TerminalInput {
HTMLCanvasElement: class HTMLCanvasElement {},
WebSocket: { OPEN: 1 },
fetch: vi.fn(),
document: { addEventListener: vi.fn(), documentElement: { dataset: {} } },
URLSearchParams,
document: { addEventListener: vi.fn(), documentElement: { dataset: {} }, getElementById: () => null },
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;
vm.runInContext(
`${read('constants.js')}\n${keyboardSource}\n${read('app.js')}\n${terminalSource}\n` +
`globalThis.__CodemanApp = CodemanApp; globalThis.__bar = KeyboardAccessoryBar;`,
context
);
const exported = context as unknown as { __CodemanApp: TerminalModule['CodemanApp']; __bar: Bar };
terminalModule = {
terminalInput: (windowStub as { CodemanTerminalInput?: TerminalInput }).CodemanTerminalInput!,
CodemanApp: exported.__CodemanApp,
bar: exported.__bar,
};
return terminalModule;
}
function loadTerminalInput(): TerminalInput {
return loadTerminalModule().terminalInput;
}
type FakeButton = {
@@ -459,6 +480,65 @@ describe('one-shot Ctrl vs terminal-generated reports', () => {
});
});
describe('one-shot Ctrl through the CJK input field', () => {
// The CJK textarea swallows keystrokes before onData sees them, so the CJK
// send path needs the modifier applied too. These drive the REAL
// _handleCjkInput against the REAL bar, both loaded into one vm scope.
function cjkApp() {
const { CodemanApp, bar } = loadTerminalModule();
bar.clearCtrl();
const app = Object.create(CodemanApp.prototype) as {
activeSessionId: string;
_sendInputAsync: ReturnType<typeof vi.fn>;
_handleCjkInput(text: string): void;
};
app.activeSessionId = 'cjk-session';
app._sendInputAsync = vi.fn();
return { app, bar };
}
it('sends the control byte for a character typed into the CJK field', () => {
const { app, bar } = cjkApp();
bar.toggleCtrl();
app._handleCjkInput('c');
expect(app._sendInputAsync).toHaveBeenCalledWith('cjk-session', '\x03');
expect(bar.isCtrlArmed()).toBe(false);
});
it('leaves ordinary CJK input untouched when nothing is armed', () => {
const { app } = cjkApp();
app._handleCjkInput('你好');
expect(app._sendInputAsync).toHaveBeenCalledWith('cjk-session', '你好');
});
it('spends the modifier on a committed IME word instead of stranding it', () => {
// The gap this closes: with the field focused the modifier could neither
// fire nor be spent, so it survived to bite a later innocent keystroke.
const { app, bar } = cjkApp();
bar.toggleCtrl();
app._handleCjkInput('你好');
expect(app._sendInputAsync).toHaveBeenCalledWith('cjk-session', '你好');
expect(bar.isCtrlArmed()).toBe(false);
});
it('spends the modifier on Enter, like every other non-character key', () => {
const { app, bar } = cjkApp();
bar.toggleCtrl();
app._handleCjkInput('\r');
expect(app._sendInputAsync).toHaveBeenCalledWith('cjk-session', '\r');
expect(bar.isCtrlArmed()).toBe(false);
});
it('drops the input, and does not spend the modifier, with no active session', () => {
const { app, bar } = cjkApp();
(app as unknown as { activeSessionId: string | null }).activeSessionId = null;
bar.toggleCtrl();
app._handleCjkInput('c');
expect(app._sendInputAsync).not.toHaveBeenCalled();
expect(bar.isCtrlArmed()).toBe(true);
});
});
describe('armed styling survives the light-skin overrides', () => {
const mobileCss = readFileSync(resolve('src/web/public/mobile.css'), 'utf8');