feat(terminal): Ctrl+C copies the selection, interrupts when nothing is selected

Closes #211. Copying from the terminal only worked through the browser
context menu, because xterm turns Ctrl+C into 0x03 and cancels the keydown,
so the muscle-memory copy failed silently and read as "no copy-paste at all".

With a selection, Ctrl+C now copies it, toasts, clears the selection and
sends nothing to the PTY. With no selection it falls through unchanged, so
the interrupt is intact. Ctrl+Shift+C is an explicit copy chord that never
falls through: an explicit copy that interrupts a running agent because the
selection happened to be empty would be a footgun.

Three details that keep the interrupt safe:

- The decision lives in attachCustomKeyEventHandler (terminal-ui.js) and the
  no-selection path returns true WITHOUT preventDefault. xterm calls the
  custom handler before its own cancel(), so returning false alone does not
  cancel the event; the copy path therefore calls preventDefault explicitly,
  or the browser would run its native copy on top of ours.
- copy-selection is a full registry entry (rebindable and disableable in App
  Settings) whose action is deliberately absent from SHORTCUT_ACTIONS, the
  same trick command-palette uses: the generic capture loop preventDefaults
  every match it dispatches, which would cost the user the interrupt key.
- The gate is keydown-only, since the custom handler also runs for keypress
  and keyup.

Copy goes through _copyText (Clipboard API, then hidden-textarea +
execCommand) rather than raw navigator.clipboard, because install.sh's LAN
option serves plain HTTP where navigator.clipboard is undefined; the
fallback steals focus, so the terminal is refocused afterwards.

Tests: test/terminal-copy-selection.test.ts pins the gate and the
SHORTCUT_ACTIONS invariant; test/terminal-copy-shortcut.test.ts drives real
key presses in chromium and asserts on the clipboard plus the bytes xterm
emitted (browser-driven, so excluded from test:ci like the other Playwright
suites). Verified manually on an isolated beta instance before landing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer
2026-08-05 02:38:57 +02:00
parent 1e5f6c8ee1
commit d9123de9eb
14 changed files with 734 additions and 2 deletions
+2
View File
@@ -52,6 +52,8 @@ describe('help modal shortcuts', () => {
});
it('documents terminal input shortcuts without advertising stale run shortcuts', () => {
expectShortcut(helpModal, ['Ctrl', 'C'], 'Copy Selection');
expectShortcut(helpModal, ['Ctrl', 'Shift', 'C'], 'Copy Selection');
expectShortcut(helpModal, ['Ctrl', 'L'], 'Clear Terminal');
expectShortcut(helpModal, ['Ctrl', '+'], 'Increase Font');
expectShortcut(helpModal, ['Ctrl', '-'], 'Decrease Font');
+18
View File
@@ -58,4 +58,22 @@ describe('keyboard shortcuts', () => {
expect(appSource).toContain('if (this.matchesShortcutEvent(e, shortcut))');
expect(appSource).toContain('if (shortcut.disabled || !shortcut.action) continue;');
});
it('keeps the interrupt when Ctrl+C copies a selection (#211)', () => {
// The xterm handler owns this decision, and the no-selection path must fall
// through with NO preventDefault so xterm still evaluates Ctrl+C into 0x03.
expect(terminalUiSource).toContain('this.shouldCopyTerminalSelectionFromShortcut?.(ev)');
expect(terminalUiSource).toMatch(
/const selection = this\.terminal\.hasSelection\?\.\(\) \? this\.terminal\.getSelection\(\) : '';/
);
expect(terminalUiSource).toContain('void this.copyTerminalSelection(selection);');
expect(appSource).toContain("id: 'copy-selection'");
});
it('documents the terminal copy shortcut in help and README', () => {
expect(helpHtml).toContain('<kbd>Ctrl</kbd>+<kbd>C</kbd>');
expect(helpHtml).toContain('<kbd>Ctrl</kbd>+<kbd>Shift</kbd>+<kbd>C</kbd>');
expect(readme).toContain('`Ctrl/Cmd+C`');
expect(readme).toContain('`Ctrl+Shift+C`');
});
});
+147
View File
@@ -0,0 +1,147 @@
/**
* Smart-copy chord gate (#211).
*
* `Ctrl+C` has to keep meaning "interrupt" whenever nothing is selected, so the
* decision is split in two: `shouldCopyTerminalSelectionFromShortcut()` only
* answers "did this chord ask to copy", and the caller in the xterm custom key
* handler decides what to do when there is no selection. These tests pin the
* gate itself (registry-aware, keydown-only) plus the static invariants that
* keep the generic capture loop from ever swallowing the interrupt.
*
* Strategy: run terminal-ui.js in a vm with a stub CodemanApp, the same harness
* shape test/command-palette-ui.test.ts uses for panels-ui.js. No DOM, no xterm.
*/
import { readFileSync } from 'node:fs';
import { resolve } from 'node:path';
import vm from 'node:vm';
import { describe, expect, it } from 'vitest';
const APP_SOURCE = readFileSync(resolve(import.meta.dirname, '../src/web/public/app.js'), 'utf8');
type Shortcut = {
id: string;
disabled?: boolean;
bindings?: Array<{ modifiers?: string[]; key?: string; code?: string }>;
};
function loadTerminalHarness(registry?: Shortcut[]) {
const CodemanApp = function CodemanApp(this: unknown) {};
const context = vm.createContext({
CodemanApp,
window: {},
document: { getElementById: () => null, querySelector: () => null },
console,
MobileDetection: { isTouchDevice: () => false, getDeviceType: () => 'desktop' },
Object,
});
const terminalUi = readFileSync(resolve(import.meta.dirname, '../src/web/public/terminal-ui.js'), 'utf8');
vm.runInContext(terminalUi, context, { filename: 'terminal-ui.js' });
const app = new (CodemanApp as unknown as new () => Record<string, any>)();
if (registry) {
app.getShortcutRegistry = () => registry;
// Real implementation, copied by reference from app.js semantics: ctrl/meta are
// interchangeable, every other modifier must be declared by the binding.
app.matchesShortcutEvent = (e: any, shortcut: Shortcut) => {
if (!shortcut || !Array.isArray(shortcut.bindings)) return false;
return shortcut.bindings.some((binding) => {
const mods = binding.modifiers || [];
const wantsPrimary = mods.includes('ctrl') || mods.includes('meta');
if (wantsPrimary !== !!(e.ctrlKey || e.metaKey)) return false;
if (mods.includes('shift') !== !!e.shiftKey) return false;
if (mods.includes('alt') !== !!e.altKey) return false;
if (binding.code && e.code === binding.code) return true;
if (binding.key && typeof e.key === 'string' && e.key.toLowerCase() === binding.key.toLowerCase()) return true;
return false;
});
};
}
return app;
}
const DEFAULT_REGISTRY: Shortcut[] = [
{
id: 'copy-selection',
bindings: [
{ modifiers: ['ctrl'], key: 'c' },
{ modifiers: ['ctrl', 'shift'], key: 'C' },
],
},
];
function keydown(over: Record<string, unknown> = {}) {
return {
type: 'keydown',
key: 'c',
code: 'KeyC',
ctrlKey: true,
shiftKey: false,
altKey: false,
metaKey: false,
...over,
};
}
describe('terminal smart-copy gate', () => {
it('matches the default Ctrl+C and Ctrl+Shift+C chords', () => {
const app = loadTerminalHarness(DEFAULT_REGISTRY);
expect(app.shouldCopyTerminalSelectionFromShortcut(keydown())).toBe(true);
expect(app.shouldCopyTerminalSelectionFromShortcut(keydown({ key: 'C', shiftKey: true }))).toBe(true);
// Cmd+C on macOS: the registry treats ctrl/meta as interchangeable.
expect(app.shouldCopyTerminalSelectionFromShortcut(keydown({ ctrlKey: false, metaKey: true }))).toBe(true);
});
it('ignores plain typing and unrelated chords', () => {
const app = loadTerminalHarness(DEFAULT_REGISTRY);
expect(app.shouldCopyTerminalSelectionFromShortcut(keydown({ ctrlKey: false }))).toBe(false);
expect(app.shouldCopyTerminalSelectionFromShortcut(keydown({ key: 'k', code: 'KeyK' }))).toBe(false);
expect(app.shouldCopyTerminalSelectionFromShortcut(keydown({ key: 'v', code: 'KeyV' }))).toBe(false);
});
it('only decides on keydown (the handler also runs for keypress and keyup)', () => {
const app = loadTerminalHarness(DEFAULT_REGISTRY);
expect(app.shouldCopyTerminalSelectionFromShortcut(keydown({ type: 'keypress' }))).toBe(false);
expect(app.shouldCopyTerminalSelectionFromShortcut(keydown({ type: 'keyup' }))).toBe(false);
expect(app.shouldCopyTerminalSelectionFromShortcut(null)).toBe(false);
});
it('honors a disabled shortcut so Ctrl+C goes back to being the interrupt', () => {
const app = loadTerminalHarness([{ ...DEFAULT_REGISTRY[0], disabled: true }]);
expect(app.shouldCopyTerminalSelectionFromShortcut(keydown())).toBe(false);
expect(app.shouldCopyTerminalSelectionFromShortcut(keydown({ key: 'C', shiftKey: true }))).toBe(false);
});
it('honors a rebound chord and stops claiming the old one', () => {
const app = loadTerminalHarness([{ id: 'copy-selection', bindings: [{ modifiers: ['alt'], key: 'y' }] }]);
expect(
app.shouldCopyTerminalSelectionFromShortcut(keydown({ ctrlKey: false, altKey: true, key: 'y', code: 'KeyY' }))
).toBe(true);
expect(app.shouldCopyTerminalSelectionFromShortcut(keydown())).toBe(false);
});
it('falls back to the default chord when no registry is available', () => {
const app = loadTerminalHarness(); // no getShortcutRegistry / matchesShortcutEvent
expect(app.shouldCopyTerminalSelectionFromShortcut(keydown())).toBe(true);
expect(app.shouldCopyTerminalSelectionFromShortcut(keydown({ key: 'x', code: 'KeyX' }))).toBe(false);
});
});
describe('smart-copy wiring invariants', () => {
it('registers copy-selection in the shortcut registry', () => {
expect(APP_SOURCE).toContain("id: 'copy-selection'");
expect(APP_SOURCE).toContain("action: 'copyTerminalSelection'");
});
it('keeps copyTerminalSelection OUT of SHORTCUT_ACTIONS', () => {
// The generic capture loop preventDefaults on every match it dispatches. If
// the copy action were reachable from there, Ctrl+C would be swallowed with
// no selection and the user would lose the interrupt key.
const actionsBlock = APP_SOURCE.slice(
APP_SOURCE.indexOf('const SHORTCUT_ACTIONS = {'),
APP_SOURCE.indexOf('// Use capture to handle before terminal')
);
expect(actionsBlock.length).toBeGreaterThan(0);
expect(actionsBlock).not.toContain('copyTerminalSelection');
});
});
+166
View File
@@ -0,0 +1,166 @@
/**
* Smart copy in a real browser (#211).
*
* The gate itself is unit-tested in test/terminal-copy-selection.test.ts. What
* can only be proven in a browser is the half that decides whether the PTY sees
* an interrupt: xterm calls the custom key handler BEFORE its own cancel(), so
* returning false does not preventDefault, and a synthetic KeyboardEvent never
* triggers a browser default action. Both facts mean the copy/interrupt split
* has to be driven with real key presses.
*
* Assertions are on real state: what landed on the clipboard, and what xterm
* emitted through onData (the bytes that would reach the PTY).
*
* Browser-driven, so it is excluded from `npm run test:ci` like the other
* Playwright suites. Run locally: npm test -- test/terminal-copy-shortcut.test.ts
*
* Port: 3174 (per MEMORY.md, ports 3150+ for tests)
*/
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 = 3174;
const BASE_URL = `http://localhost:${PORT}`;
describe('terminal Ctrl+C smart copy', () => {
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 });
const context = await browser.newContext({ permissions: ['clipboard-read', 'clipboard-write'] });
page = await context.newPage();
await page.goto(BASE_URL, { waitUntil: 'domcontentloaded' });
await page.waitForFunction(() => (window as any).app?.terminal, null, { timeout: 30000 });
// The first write after load can be dropped while the app finishes wiring
// its render pipeline, so poll until one really lands in the buffer.
await page.waitForFunction(
async () => {
const term = (window as any).app.terminal;
await new Promise((r) => term.write('\r\nWARMUP\r\n', r));
const buf = term.buffer.active;
for (let i = 0; i < buf.length; i++) {
if (buf.getLine(i)?.translateToString(true).includes('WARMUP')) return true;
}
return false;
},
null,
{ timeout: 20000, polling: 500 }
);
}, 90000);
afterAll(async () => {
if (browser) await browser.close();
if (server) await server.stop();
}, 60000);
/** Write a marker line, optionally select it, and reset the capture state. */
async function setup(line: string, select: boolean, overrides: Record<string, unknown> = {}) {
await page.evaluate(
async ({ line, select, overrides }) => {
const app = (window as any).app;
const term = app.terminal;
const settings = app.loadAppSettingsFromStorage();
settings.shortcutOverrides = overrides;
app.saveAppSettingsToStorage(settings);
(window as any).__data = [];
if (!(window as any).__dataHooked) {
term.onData((d: string) => (window as any).__data.push(d));
(window as any).__dataHooked = true;
}
await new Promise((r) => term.write('\r\n' + line + '\r\n', r));
term.clearSelection();
if (select) {
const buf = term.buffer.active;
let row = -1;
for (let i = 0; i < buf.length; i++) {
if (buf.getLine(i)?.translateToString(true).includes(line)) row = i;
}
if (row === -1) throw new Error('marker line not found in buffer');
term.select(0, row, line.length);
if (!(term.getSelection() || '').trim()) throw new Error('selection is empty');
}
document.querySelector('.xterm-helper-textarea')!.dispatchEvent(new Event('focus'));
(document.querySelector('.xterm-helper-textarea') as HTMLElement).focus();
await navigator.clipboard.writeText('SENTINEL');
},
{ line, select, overrides }
);
}
async function outcome() {
await page.waitForTimeout(350);
return page.evaluate(async () => ({
data: (window as any).__data as string[],
clipboard: (await navigator.clipboard.readText()).trim(),
hasSelection: (window as any).app.terminal.hasSelection(),
}));
}
it('copies the selection and sends nothing to the PTY', async () => {
await setup('COPY-CASE-SELECTED', true);
await page.keyboard.press('Control+c');
const res = await outcome();
expect(res.clipboard).toBe('COPY-CASE-SELECTED');
expect(res.data).toEqual([]);
expect(res.hasSelection).toBe(false); // cleared, so a second Ctrl+C interrupts
});
it('still interrupts when nothing is selected', async () => {
await setup('COPY-CASE-UNSELECTED', false);
await page.keyboard.press('Control+c');
const res = await outcome();
expect(res.data).toEqual(['\x03']);
expect(res.clipboard).toBe('SENTINEL');
});
it('copies on the explicit Ctrl+Shift+C chord', async () => {
await setup('COPY-CASE-EXPLICIT', true);
await page.keyboard.press('Control+Shift+C');
const res = await outcome();
expect(res.clipboard).toBe('COPY-CASE-EXPLICIT');
expect(res.data).toEqual([]);
});
it('never interrupts on Ctrl+Shift+C with an empty selection', async () => {
await setup('COPY-CASE-EXPLICIT-EMPTY', false);
await page.keyboard.press('Control+Shift+C');
const res = await outcome();
expect(res.data).toEqual([]);
expect(res.clipboard).toBe('SENTINEL');
});
it('restores the plain interrupt when the shortcut is disabled', async () => {
await setup('COPY-CASE-DISABLED', true, { 'copy-selection': { disabled: true } });
await page.keyboard.press('Control+c');
const res = await outcome();
expect(res.data).toEqual(['\x03']);
expect(res.clipboard).toBe('SENTINEL');
});
it('follows a rebind, and Ctrl+C goes back to pure interrupt', async () => {
await setup('COPY-CASE-REBOUND', true, { 'copy-selection': { bindings: [{ modifiers: ['alt'], key: 'y' }] } });
await page.keyboard.press('Alt+y');
const rebound = await outcome();
expect(rebound.clipboard).toBe('COPY-CASE-REBOUND');
expect(rebound.data).toEqual([]);
await setup('COPY-CASE-REBOUND-2', true, { 'copy-selection': { bindings: [{ modifiers: ['alt'], key: 'y' }] } });
await page.keyboard.press('Control+c');
const res = await outcome();
expect(res.data).toEqual(['\x03']);
});
it('leaves Ctrl+V on the paste trap', async () => {
await setup('COPY-CASE-PASTE', false);
await page.keyboard.press('Control+v');
const res = await outcome();
expect(res.data.join('')).toContain('SENTINEL'); // pasted text, not ^V
expect(res.data.join('')).not.toContain('\x16');
});
});