Merge pull request #522 from opticon454/fix/newline-sequence-capability

feat(terminal): newline chord as registry data, plus a Key tester in Settings # Conflicts: # config/test-suites.ts
2026-10-05 15:09:42 +02:00 · 2026-10-04 23:24:00 +02:00
parent d887002ca8 2cf37529e9
commit c197e9370b
13 changed files with 288 additions and 12 deletions
@@ -0,0 +1,37 @@
+// @vitest-environment node
+// capabilities.newline: the bytes Shift+Enter types into a CLI's pane. Data in the registry, not
+// a branch on the CLI id (test/cli-registry-no-id-branching.test.ts keeps the latter true).
+
+import { describe, expect, it } from 'vitest';
+import { CliEntrySchema } from '../src/config/cli-registry/schema.js';
+import { STOCK_CLIS } from '../src/config/cli-registry/stock.js';
+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('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) => {
+    const e = claude();
+    (e.capabilities as Record<string, unknown>).newline = value;
+    expect(CliEntrySchema.safeParse(e).success).toBe(true);
+  });
+
+  it.each(['lf', 'crlf', '\x1b\r', '', 0])('schema rejects %j (no free-form byte strings in config)', (value) => {
+    const e = claude();
+    (e.capabilities as Record<string, unknown>).newline = value;
+    expect(CliEntrySchema.safeParse(e).success).toBe(false);
+  });
+
+  it('is optional, so an entry that declares nothing keeps the line feed', () => {
+    const e = claude();
+    delete (e.capabilities as Record<string, unknown>).newline;
+    expect(CliEntrySchema.safeParse(e).success).toBe(true);
+  });
+});
@@ -0,0 +1,96 @@
+/** @fileoverview Settings → Terminal & Input → Key tester, driven with real keystrokes in Chromium. */
+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 = 3194;
+
+describe('Key tester in a real browser', () => {
+  let server: WebServer;
+  let browser: Browser;
+  let page: Page;
+
+  beforeAll(async () => {
+    server = new WebServer(PORT, false, true);
+    await server.start();
+    browser = await chromium.launch({ headless: true });
+    page = await browser.newPage();
+    await page.goto(`http://localhost:${PORT}`, { waitUntil: 'domcontentloaded' });
+    await page.waitForFunction(() => (window as any).app?.terminal, null, { timeout: 30000 });
+    await page.evaluate(() => (window as any).app.openAppSettings());
+    await page.focus('#keyTesterInput');
+  }, 90000);
+
+  afterAll(async () => {
+    if (browser) await browser.close();
+    if (server) await server.stop();
+  }, 60000);
+
+  const log = () => page.evaluate(() => document.getElementById('keyTesterLog')!.textContent ?? '');
+
+  it('shows keydown, keypress and keyup for Shift+Enter, with the modifier and charCode', async () => {
+    await page.keyboard.press('Shift+Enter');
+    const text = await log();
+    expect(text).toMatch(/keydown\s+key="Enter" code=Enter mods=shift/);
+    // The keypress is the event that used to leak a bare \r to the PTY.
+    expect(text).toMatch(/keypress\s+key="Enter" code=Enter mods=shift charCode=13/);
+    expect(text).toMatch(/keyup\s+key="Enter" code=Enter mods=shift/);
+  });
+
+  it('shows Ctrl+Enter without a keypress, as xterm would never see one for Ctrl', async () => {
+    await page.evaluate(() => (document.getElementById('keyTesterLog')!.textContent = ''));
+    await page.keyboard.press('Control+Enter');
+    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 () => {
+    for (let i = 0; i < 8; i++) await page.keyboard.press('a');
+    expect((await log()).split('\n').length).toBeLessThanOrEqual(14);
+    expect(await page.inputValue('#keyTesterInput')).toBe('');
+  });
+});
@@ -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';
@@ -165,6 +167,74 @@ describe('session-routes', () => {
      expect(argv).toContain('-H');
    });

+    describe('newline chord comes from the CLI registry (capabilities.newline)', () => {
+      const sentHex = async (mode: string, key: string): Promise<string[]> => {
+        execFile.mockReset();
+        execFile.mockImplementation((_bin: string, _argv: string[], _opts: unknown, cb: (e: Error | null) => void) =>
+          cb(null)
+        );
+        const session = harness.ctx._session as unknown as { mode: string };
+        const before = session.mode;
+        session.mode = mode;
+        try {
+          const res = await harness.app.inject({
+            method: 'POST',
+            url: '/api/sessions/test-session-1/send-key',
+            payload: { key },
+          });
+          expect(res.statusCode).toBe(200);
+        } finally {
+          session.mode = before;
+        }
+        const argv = execFile.mock.calls[0][1] as string[];
+        return argv.slice(argv.indexOf('-H') + 3); // after "-H -t <pane>"
+      };
+
+      it('sends a line feed for Shift+Enter to a CLI that declares nothing', async () => {
+        expect(await sentHex('claude', 'S-Enter')).toEqual(['0a']);
+        expect(await sentHex('opencode', 'S-Enter')).toEqual(['0a']);
+      });
+
+      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 () => {
+        expect(await sentHex('codex', 'C-Enter')).toEqual(['0a']);
+        expect(await sentHex('claude', 'C-Enter')).toEqual(['0a']);
+      });
+
+      it('falls back to a line feed for a mode the registry does not know', async () => {
+        expect(await sentHex('no-such-cli', 'S-Enter')).toEqual(['0a']);
+      });
+    });
+
    it('rejects keys outside the hex allowlist without invoking tmux', async () => {
      execFile.mockReset();
      const res = await harness.app.inject({