mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-05 06:59:42 +02:00
fix(terminal): address review: Key tester isolates shortcuts, Codex stays on line feed
- app.js: the shortcut dispatcher returns early for events aimed at a data-raw-keys field, so Ctrl+W / Ctrl+L / Escape / Alt+1 / Ctrl+K pressed in the Key tester no longer kill the session, clear the terminal or close Settings - stock.ts: drop Codex's esc-enter (a line feed works); no stock CLI declares a chord. The esc-enter path is tested through a clis.json override - tests: unused port (3194), Ctrl+Enter asserts no keypress, shortcut-isolation test (verified to fail without the guard) - docs/comments point at capabilities.newline; set-input class, trailing whitespace Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5.5
parent
f39c66e4e8
commit
2cf37529e9
@@ -2,4 +2,4 @@
|
||||
"aicodeman": patch
|
||||
---
|
||||
|
||||
Shift+Enter's newline chord is now registry data (`capabilities.newline`: `line-feed` by default, `esc-enter` for Codex) instead of being chosen in the `send-key` route, so a CLI with a different composer is one line in `stock.ts`. Adds a Key tester under Settings → Terminal & Input that shows the keydown/keypress/keyup events a browser reports, to diagnose a device where a shortcut behaves differently.
|
||||
Shift+Enter's newline chord is now registry data (`capabilities.newline`: `line-feed` by default, `esc-enter` available for a CLI whose composer ignores a bare line feed; no stock CLI changes) instead of being chosen in the `send-key` route. Adds a Key tester under Settings → Terminal & Input that shows the keydown/keypress/keyup events a browser reports, to diagnose a device where a shortcut behaves differently. Keys pressed in the tester no longer trigger app shortcuts (Ctrl+W, Ctrl+L, Escape, ...).
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -120,7 +120,7 @@ sure its row is one the agent cannot write.
|
||||
|
||||
## The newline chord
|
||||
|
||||
`capabilities.newline` (`'line-feed'` | `'esc-enter'`, absent = line feed) is the byte sequence the `send-key` route types into the pane for Shift+Enter. A line feed (`0x0a`, also Ctrl+Enter) is what Claude Code's Ink input reads as "insert a newline"; `esc-enter` (`ESC CR`, the Option/Alt+Enter chord) is for a composer that ignores a bare line feed, which Codex does in some terminals (#495). It is an enum rather than a byte string on purpose: config never carries bytes that get typed into a pane. Settings → Terminal & Input → **Key tester** prints what a browser reports for keydown/keypress/keyup, to see whether a device is sending what you think.
|
||||
`capabilities.newline` (`'line-feed'` | `'esc-enter'`, absent = line feed) is the byte sequence the `send-key` route types into the pane for Shift+Enter. A line feed (`0x0a`, also Ctrl+Enter) is what Claude Code's Ink input reads as "insert a newline"; `esc-enter` (`ESC CR`, the Option/Alt+Enter chord) is there for a composer that ignores a bare line feed. No stock CLI declares it today: the bytes are typed by tmux on the server, so the browser's OS cannot change what a CLI reads, and Codex 0.147.0 was checked to take a line feed (a Shift+Enter that submits is the keypress leak fixed in #520, not a byte problem). A user `clis.json` can set it for a CLI that needs it. It is an enum rather than a byte string on purpose: config never carries bytes that get typed into a pane. Settings → Terminal & Input → **Key tester** prints what a browser reports for keydown/keypress/keyup, to see whether a device is sending what you think.
|
||||
|
||||
## Arg-template safety
|
||||
|
||||
|
||||
@@ -620,9 +620,6 @@ const CODEX: CliEntry = {
|
||||
// `dangerouslyBypassApprovals` on the wire), so it is the one that would have caught a
|
||||
// regression; `schema.ts` now rejects a name that is not a declared param.
|
||||
privilegedParams: [{ param: 'bypassApprovals', clampTo: false }],
|
||||
// Codex's composer ignores a bare line feed in some terminals (Windows browsers, #495); the
|
||||
// Esc+Enter chord is the one its own Option+Enter uses.
|
||||
newline: 'esc-enter',
|
||||
// Verified by hand against a real llama.cpp server. Written to an isolated CODEX_HOME
|
||||
// so the user's real ~/.codex/config.toml is never touched.
|
||||
customModelInjection: {
|
||||
|
||||
@@ -1242,6 +1242,12 @@ class CodemanApp {
|
||||
|
||||
// Use capture to handle before terminal
|
||||
document.addEventListener('keydown', (e) => {
|
||||
// A field that exists to show what a key does (Settings → Key tester, `data-raw-keys`) must
|
||||
// receive every chord untouched. Without this, probing Ctrl+W killed the active session,
|
||||
// Ctrl+L cleared the terminal and Escape closed Settings: this listener runs in the capture
|
||||
// phase, before the field's own handler. Must stay the first statement.
|
||||
if (e.target?.closest?.('[data-raw-keys]')) return;
|
||||
|
||||
// Don't intercept keys during CJK IME composition
|
||||
if (e.isComposing || e.keyCode === 229) return;
|
||||
|
||||
|
||||
@@ -1822,7 +1822,7 @@
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
|
||||
<div class="set-group">
|
||||
<div class="set-group-head"><h4>Key tester</h4><span class="set-scope">device</span></div>
|
||||
<div class="set-group-body">
|
||||
@@ -1831,7 +1831,7 @@
|
||||
<span class="set-row-label">Key tester</span>
|
||||
<span class="set-row-desc">Click the box and press keys to see what this browser reports (key, code, modifiers) for keydown, keypress and keyup. Useful when a shortcut such as Shift+Enter behaves differently on one device. Nothing is sent to a session.</span>
|
||||
</div>
|
||||
<input type="text" id="keyTesterInput" class="set-select" readonly autocomplete="off" spellcheck="false"
|
||||
<input type="text" id="keyTesterInput" class="set-input" data-raw-keys readonly autocomplete="off" spellcheck="false"
|
||||
placeholder="Click here, then press keys"
|
||||
onkeydown="app.keyTesterEvent(event)" onkeypress="app.keyTesterEvent(event)" onkeyup="app.keyTesterEvent(event)">
|
||||
</div>
|
||||
|
||||
@@ -2093,8 +2093,9 @@ export function registerSessionRoutes(
|
||||
|
||||
// ========== Send Named Key (tmux send-keys -H) ==========
|
||||
// Sends raw hex bytes to tmux pane for keys like Shift+Enter / Ctrl+Enter.
|
||||
// Uses send-keys -H (hex) to inject 0x0a (line feed) which Claude Code's
|
||||
// Ink input recognizes as "insert newline" vs 0x0d (carriage return = submit).
|
||||
// Uses send-keys -H (hex) to inject a newline chord: 0x0a (line feed) by default, or the CLI's
|
||||
// own `capabilities.newline`. Claude Code's Ink input recognizes 0x0a as "insert newline" vs
|
||||
// 0x0d (carriage return = submit).
|
||||
|
||||
app.post('/api/sessions/:id/send-key', async (req) => {
|
||||
const { id } = req.params as { id: string };
|
||||
|
||||
@@ -10,11 +10,11 @@ import type { CliEntry } from '../src/config/cli-registry/types.js';
|
||||
const claude = () => structuredClone(STOCK_CLIS.find((e) => (e.id as string) === 'claude')!) as CliEntry;
|
||||
|
||||
describe('capabilities.newline', () => {
|
||||
it('only codex declares a non-default chord today', () => {
|
||||
const declared = Object.fromEntries(
|
||||
STOCK_CLIS.filter((e) => e.capabilities.newline).map((e) => [e.id as string, e.capabilities.newline])
|
||||
);
|
||||
expect(declared).toEqual({ codex: 'esc-enter' });
|
||||
it('no stock CLI declares a chord: every one keeps the line feed', () => {
|
||||
// codex 0.147.0 takes a line feed (checked against a real tmux pane), so there is no CLI that
|
||||
// needs esc-enter yet. The capability exists for a user clis.json override and the next CLI.
|
||||
const declared = STOCK_CLIS.filter((e) => e.capabilities.newline).map((e) => e.id as string);
|
||||
expect(declared).toEqual([]);
|
||||
});
|
||||
|
||||
it.each(['line-feed', 'esc-enter'])('schema accepts %s', (value) => {
|
||||
|
||||
@@ -3,7 +3,7 @@ import { describe, it, expect, beforeAll, afterAll } from 'vitest';
|
||||
import { chromium, type Browser, type Page } from 'playwright';
|
||||
import { WebServer } from '../src/web/server.js';
|
||||
|
||||
const PORT = 3197;
|
||||
const PORT = 3194;
|
||||
|
||||
describe('Key tester in a real browser', () => {
|
||||
let server: WebServer;
|
||||
@@ -43,6 +43,49 @@ describe('Key tester in a real browser', () => {
|
||||
const text = await log();
|
||||
expect(text).toMatch(/keydown\s+key="Enter" code=Enter mods=ctrl/);
|
||||
expect(text).toMatch(/keyup/);
|
||||
// Chromium emits no keypress for a Ctrl chord, which is why only Shift+Enter ever leaked a \r.
|
||||
expect(text).not.toMatch(/keypress/);
|
||||
});
|
||||
|
||||
it('lets no app shortcut fire for keys pressed in the field (Ctrl+W, Ctrl+L, Escape, Alt+1, Ctrl+K)', async () => {
|
||||
// The shortcut dispatcher is a capture-phase document listener, so without a guard it ran before
|
||||
// the field's own handler: Ctrl+W killed the active session, Ctrl+L cleared the terminal and
|
||||
// Escape closed Settings, while this row says nothing is sent to a session.
|
||||
await page.evaluate(() => {
|
||||
const app = (window as any).app;
|
||||
const calls: string[] = [];
|
||||
(window as any).__calls = calls;
|
||||
for (const name of ['killActiveSession', 'clearTerminal', 'openCommandPalette', 'closeAllPanels']) {
|
||||
app[name] = (...args: unknown[]) => void calls.push(name + args.length);
|
||||
}
|
||||
});
|
||||
await page.focus('#keyTesterInput');
|
||||
// [chord, what the tester must report for it]; checked one at a time because the log keeps 14 lines.
|
||||
const chords: [string, RegExp][] = [
|
||||
['Control+W', /key="w" code=KeyW mods=ctrl/i],
|
||||
['Control+L', /key="l" code=KeyL mods=ctrl/i],
|
||||
['Escape', /key="Escape" code=Escape/],
|
||||
['Alt+1', /code=Digit1 mods=alt/],
|
||||
['Control+K', /key="k" code=KeyK mods=ctrl/i],
|
||||
];
|
||||
for (const [chord, seen] of chords) {
|
||||
await page.evaluate(() => (document.getElementById('keyTesterLog')!.textContent = ''));
|
||||
await page.keyboard.press(chord);
|
||||
expect(await log(), chord).toMatch(seen);
|
||||
expect(await page.evaluate(() => (window as any).__calls), chord).toEqual([]);
|
||||
}
|
||||
expect(await page.evaluate(() => document.getElementById('appSettingsModal')!.classList.contains('active'))).toBe(
|
||||
true
|
||||
);
|
||||
});
|
||||
|
||||
it('still lets the shortcut fire anywhere else (the guard is scoped to data-raw-keys)', async () => {
|
||||
await page.evaluate(() => {
|
||||
(window as any).__calls.length = 0;
|
||||
(document.activeElement as HTMLElement | null)?.blur();
|
||||
});
|
||||
await page.keyboard.press('Escape');
|
||||
expect(await page.evaluate(() => (window as any).__calls)).toContain('closeAllPanels0');
|
||||
});
|
||||
|
||||
it('keeps only the last 14 lines and never types into the field', async () => {
|
||||
|
||||
@@ -17,7 +17,9 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
|
||||
import Fastify, { type FastifyInstance } from 'fastify';
|
||||
import fastifyCookie from '@fastify/cookie';
|
||||
import fastifyMultipart from '@fastify/multipart';
|
||||
import { join } from 'node:path';
|
||||
import { dirname, join } from 'node:path';
|
||||
import { mkdirSync, rmSync, writeFileSync } from 'node:fs';
|
||||
import { registryFilePath, reloadCliRegistry } from '../../src/config/cli-registry/registry.js';
|
||||
import { mkdtemp, rm, mkdir, writeFile } from 'node:fs/promises';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { createMockRouteContext, type MockRouteContext } from '../mocks/index.js';
|
||||
@@ -193,8 +195,34 @@ describe('session-routes', () => {
|
||||
expect(await sentHex('opencode', 'S-Enter')).toEqual(['0a']);
|
||||
});
|
||||
|
||||
it('sends Esc+Enter for Shift+Enter to a CLI that declares esc-enter', async () => {
|
||||
expect(await sentHex('codex', 'S-Enter')).toEqual(['1b', '0d']);
|
||||
it('sends a line feed to Codex too: no stock CLI declares a chord', async () => {
|
||||
expect(await sentHex('codex', 'S-Enter')).toEqual(['0a']);
|
||||
});
|
||||
|
||||
describe('a CLI that declares esc-enter (here via a user clis.json override of codex)', () => {
|
||||
beforeEach(() => {
|
||||
const file = registryFilePath();
|
||||
mkdirSync(dirname(file), { recursive: true });
|
||||
writeFileSync(
|
||||
file,
|
||||
JSON.stringify({ schemaVersion: 1, clis: { codex: { capabilities: { newline: 'esc-enter' } } } }),
|
||||
{ mode: 0o600 }
|
||||
);
|
||||
reloadCliRegistry();
|
||||
});
|
||||
afterEach(() => {
|
||||
rmSync(registryFilePath(), { force: true });
|
||||
reloadCliRegistry();
|
||||
});
|
||||
|
||||
it('sends Esc+Enter for Shift+Enter, and only to that CLI', async () => {
|
||||
expect(await sentHex('codex', 'S-Enter')).toEqual(['1b', '0d']);
|
||||
expect(await sentHex('claude', 'S-Enter')).toEqual(['0a']);
|
||||
});
|
||||
|
||||
it('still sends a line feed for Ctrl+Enter', async () => {
|
||||
expect(await sentHex('codex', 'C-Enter')).toEqual(['0a']);
|
||||
});
|
||||
});
|
||||
|
||||
it('always sends a line feed for Ctrl+Enter', async () => {
|
||||
|
||||
Reference in New Issue
Block a user