From 1aeabdfd551fe898c549839dbdb19519a5794b2f Mon Sep 17 00:00:00 2001 From: Devvyn <22340871+opticon454@users.noreply.github.com> Date: Sat, 10 Oct 2026 12:55:01 +0800 Subject: [PATCH 01/50] feat(shortcuts): rebindable previous/next session, duplicate-chord refusal, rebinds stay out of the PTY --- .changeset/keybinding-polish.md | 5 ++ src/web/public/app.js | 82 +++++++++++++++++++--------- src/web/public/i18n.js | 2 + src/web/public/settings-ui.js | 32 +++++++++++ src/web/public/terminal-tile.js | 1 + src/web/public/terminal-ui.js | 3 + test/keyboard-shortcuts.test.ts | 21 +++++-- test/terminal-copy-selection.test.ts | 4 +- 8 files changed, 116 insertions(+), 34 deletions(-) create mode 100644 .changeset/keybinding-polish.md diff --git a/.changeset/keybinding-polish.md b/.changeset/keybinding-polish.md new file mode 100644 index 00000000..2ca2e0b0 --- /dev/null +++ b/.changeset/keybinding-polish.md @@ -0,0 +1,5 @@ +--- +'aicodeman': minor +--- + +Shortcut rebinding (App Settings → Shortcuts) now covers Previous Session and the alternate Next Session, which were fixed at Alt/Option+[ and Alt/Option+]. Capturing a chord another action already owns is refused with a message naming that action (Alt+1-9 are reserved for tab switching). A chord you rebound to an app action no longer also types into the terminal: a rebind to Alt+J used to switch tabs and send ESC j to the session as well. diff --git a/src/web/public/app.js b/src/web/public/app.js index dda35644..0cd59388 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -637,11 +637,20 @@ const DEFAULT_SHORTCUTS = [ bindings: [], action: 'removeTile', }, + // Physical key codes, so macOS Option layouts that print "“" for Option+[ still match. { - id: 'previous-next-session', + id: 'previous-session', group: 'Session', - label: 'Previous / Next Session', - displayBindings: ['Alt/Option+[', 'Alt/Option+]'], + label: 'Previous Session', + bindings: [{ modifiers: ['alt'], code: 'BracketLeft' }], + action: 'previousSession', + }, + { + id: 'next-session-alt', + group: 'Session', + label: 'Next Session (alternate)', + bindings: [{ modifiers: ['alt'], code: 'BracketRight' }], + action: 'nextSession', }, { id: 'switch-tab-n', @@ -1378,19 +1387,7 @@ class CodemanApp { // getShortcutRegistry()). The command palette chord is deliberately NOT in // this map — shouldOpenCommandPaletteFromShortcut() dispatches it above with // focus-target awareness (it must fire from the terminal but not from inputs). - const SHORTCUT_ACTIONS = { - showShortcutOverlay: () => this.showShortcutOverlay(), - killActiveSession: () => this.killActiveSession(), - nextSession: () => this.nextSession(), - clearTerminal: () => this.clearTerminal(), - restoreTerminalSize: () => this.restoreTerminalSize(), - increaseFontSize: () => this.increaseFontSize(), - decreaseFontSize: () => this.decreaseFontSize(), - toggleVoiceInput: () => VoiceInput.toggle(), - moveActiveTabLeft: () => this.moveActiveTabLeft(), - moveActiveTabRight: () => this.moveActiveTabRight(), - toggleSessionSidebar: () => this.toggleSessionSidebar(), - }; + const SHORTCUT_ACTIONS = this._shortcutActions(); // Use capture to handle before terminal document.addEventListener('keydown', (e) => { @@ -1453,7 +1450,7 @@ class CodemanApp { // Option/Alt session navigation uses physical key CODES, not e.key, so macOS // keyboard layouts that emit special characters under Option (Option+1 -> ¡, // Option+[ -> "“") still switch sessions. e.code is the physical key regardless - // of layout. Option+1-9 = switch by index; Option+[ / Option+] = prev / next. + // of layout. Option+1-9 = switch by index. Option+[ / ] (prev / next) live in the shortcut registry, so they can be rebound. if (e.altKey && !e.ctrlKey && !e.shiftKey) { const code = e.code || ''; const digitMatch = code.match(/^Digit([1-9])$/); @@ -1479,16 +1476,6 @@ class CodemanApp { } return; } - if (e.code === 'BracketLeft') { - e.preventDefault(); - this.prevSession(); - return; - } - if (e.code === 'BracketRight') { - e.preventDefault(); - this.nextSession(); - return; - } } // Tile grid chords: only where they apply (tile-grid.js tileShortcutFor), @@ -10203,6 +10190,47 @@ class CodemanApp { } // ─── Shortcut Registry ─────────────────────────────────────────────────────── + /** + * Action name -> handler for the shortcut registry entries the document keydown + * handler dispatches generically (DEFAULT_SHORTCUTS + the user's rebinds). Entries + * absent here (copy selection, the palette, the tile chords) have their own dispatch. + */ + _shortcutActions() { + return { + showShortcutOverlay: () => this.showShortcutOverlay(), + killActiveSession: () => this.killActiveSession(), + nextSession: () => this.nextSession(), + previousSession: () => this.prevSession(), + clearTerminal: () => this.clearTerminal(), + restoreTerminalSize: () => this.restoreTerminalSize(), + increaseFontSize: () => this.increaseFontSize(), + decreaseFontSize: () => this.decreaseFontSize(), + toggleVoiceInput: () => VoiceInput.toggle(), + moveActiveTabLeft: () => this.moveActiveTabLeft(), + moveActiveTabRight: () => this.moveActiveTabRight(), + toggleSessionSidebar: () => this.toggleSessionSidebar(), + }; + } + + /** + * True when `e` is a chord the USER rebound to a generically dispatched action. + * The document handler has already run the action, but its preventDefault() does not + * stop xterm, so without this gate a rebind to e.g. Alt+J would also type ESC j into the + * session. Defaults are not covered on purpose: several (Ctrl+L) rely on reaching the PTY. + */ + isUserBoundShortcutEvent(e) { + if (!e.ctrlKey && !e.metaKey && !e.altKey) return false; + const overrides = this.loadAppSettingsFromStorage().shortcutOverrides || {}; + const actions = this._shortcutActions(); + return this.getShortcutRegistry().some( + (shortcut) => + !shortcut.disabled && + Array.isArray(overrides[shortcut.id]?.bindings) && + Object.hasOwn(actions, shortcut.action) && + this.matchesShortcutEvent(e, shortcut) + ); + } + // Returns the merged shortcut list: DEFAULT_SHORTCUTS with any per-shortcut // overrides from settings.shortcutOverrides applied on top. diff --git a/src/web/public/i18n.js b/src/web/public/i18n.js index f4f92703..ff997009 100644 --- a/src/web/public/i18n.js +++ b/src/web/public/i18n.js @@ -1140,6 +1140,8 @@ 'Close Panels': '关闭面板', 'Previous / Next Session': '上一个 / 下一个会话', 'Next Session': '下一个会话', + 'Previous Session': '上一个会话', + 'Next Session (alternate)': '下一个会话(备用)', 'Switch to Tab N': '切换到第 N 个标签页', 'Move Active Tab Left': '向左移动当前标签页', 'Move Active Tab Right': '向右移动当前标签页', diff --git a/src/web/public/settings-ui.js b/src/web/public/settings-ui.js index 275b8eff..d1d43503 100644 --- a/src/web/public/settings-ui.js +++ b/src/web/public/settings-ui.js @@ -4704,6 +4704,14 @@ Object.assign(CodemanApp.prototype, { this.showToast?.('Shortcut must include Ctrl, Cmd, or Alt', 'error'); return; } + // Refuse a chord something else already owns: the registry dispatches the first match, + // so a duplicate would silently take over (or lose to) the other action. + const taken = this.findShortcutConflict(e, shortcutId); + if (taken) { + this.renderShortcutSettingsList(); + this.showToast?.(`${this.describeShortcutEvent(e)} is already used by ${taken}`, 'error'); + return; + } const modifiers = []; if (e.ctrlKey) modifiers.push('ctrl'); if (e.metaKey) modifiers.push('meta'); @@ -4720,6 +4728,30 @@ Object.assign(CodemanApp.prototype, { this.renderShortcutSettingsList(); }, + /** Label of the action that already owns the chord in `e`, or '' when it is free. */ + findShortcutConflict(e, shortcutId) { + // Alt+1-9 switch tabs by index; they are fixed, not in the registry. + if (e.altKey && !e.ctrlKey && !e.metaKey && !e.shiftKey && /^Digit[1-9]$/.test(e.code || '')) { + return 'Switch to Tab N'; + } + if (typeof this.getShortcutRegistry !== 'function' || typeof this.matchesShortcutEvent !== 'function') return ''; + const other = this.getShortcutRegistry().find( + (shortcut) => + shortcut.id !== shortcutId && !shortcut.disabled && Array.isArray(shortcut.bindings) && this.matchesShortcutEvent(e, shortcut) + ); + return other ? other.label : ''; + }, + + describeShortcutEvent(e) { + const parts = []; + if (e.ctrlKey) parts.push('Ctrl'); + if (e.metaKey) parts.push('Meta'); + if (e.shiftKey) parts.push('Shift'); + if (e.altKey) parts.push('Alt'); + parts.push(e.key && e.key.length === 1 ? e.key.toUpperCase() : e.key || e.code); + return parts.join('+'); + }, + resetShortcutOverride(shortcutId) { const settings = this.loadAppSettingsFromStorage(); const shortcutOverrides = { ...(settings.shortcutOverrides || {}) }; diff --git a/src/web/public/terminal-tile.js b/src/web/public/terminal-tile.js index 5ac18139..e31596bc 100644 --- a/src/web/public/terminal-tile.js +++ b/src/web/public/terminal-tile.js @@ -321,6 +321,7 @@ ) { return false; } + if (ev.type === 'keydown' && global.app?.isUserBoundShortcutEvent?.(ev)) return false; if (ev.type === 'keydown' && global.app?.shouldOpenCommandPaletteFromShortcut?.(ev)) { return false; } diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index 5a8f659b..11c08f41 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -653,6 +653,9 @@ Object.assign(CodemanApp.prototype, { return false; } + // A chord the user rebound to an app action (Settings -> Shortcuts) must not also type into the PTY. + if (ev.type === 'keydown' && this.isUserBoundShortcutEvent?.(ev)) return false; + // Command palette chord (COD-153): keep it out of the PTY. The document // CAPTURE handler has already opened the palette by the time xterm sees // this keydown, but its preventDefault() does NOT stop xterm — without diff --git a/test/keyboard-shortcuts.test.ts b/test/keyboard-shortcuts.test.ts index 00a8a1ac..288d9885 100644 --- a/test/keyboard-shortcuts.test.ts +++ b/test/keyboard-shortcuts.test.ts @@ -14,10 +14,11 @@ describe('keyboard shortcuts', () => { }); it('provides Option+bracket shortcuts for previous and next session', () => { - expect(appSource).toContain("e.code === 'BracketLeft'"); - expect(appSource).toContain("e.code === 'BracketRight'"); - expect(appSource).toContain('this.prevSession()'); - expect(appSource).toContain('this.nextSession()'); + // Registry entries (so they can be rebound), matched by physical key code. + expect(appSource).toMatch(/id: 'previous-session'[\s\S]*?code: 'BracketLeft'[\s\S]*?action: 'previousSession'/); + expect(appSource).toMatch(/id: 'next-session-alt'[\s\S]*?code: 'BracketRight'[\s\S]*?action: 'nextSession'/); + expect(appSource).toContain('previousSession: () => this.prevSession()'); + expect(appSource).toContain('nextSession: () => this.nextSession()'); }); it('suppresses xterm PTY injection for the same physical Alt nav codes (no ESC leak)', () => { @@ -53,7 +54,7 @@ describe('keyboard shortcuts', () => { // getShortcutRegistry() + matchesShortcutEvent() so overrides and per-shortcut // disables (App Settings → Shortcuts) actually take effect. expect(appSource).not.toContain('const SHORTCUTS = ['); - expect(appSource).toContain('const SHORTCUT_ACTIONS = {'); + expect(appSource).toContain('const SHORTCUT_ACTIONS = this._shortcutActions();'); expect(appSource).toContain('for (const shortcut of this.getShortcutRegistry())'); expect(appSource).toContain('if (this.matchesShortcutEvent(e, shortcut))'); expect(appSource).toContain('if (shortcut.disabled || !shortcut.action) continue;'); @@ -84,4 +85,14 @@ describe('keyboard shortcuts', () => { expect(readme).toContain('`Ctrl/Cmd+C`'); expect(readme).toContain('`Ctrl+Shift+C`'); }); + + it('keeps a user-rebound chord out of the PTY and refuses duplicate bindings', () => { + const tile = readFileSync('src/web/public/terminal-tile.js', 'utf8'); + const settings = readFileSync('src/web/public/settings-ui.js', 'utf8'); + expect(appSource).toContain('isUserBoundShortcutEvent(e) {'); + expect(terminalUiSource).toContain('this.isUserBoundShortcutEvent?.(ev)) return false'); + expect(tile).toContain('global.app?.isUserBoundShortcutEvent?.(ev)) return false'); + expect(settings).toContain('this.findShortcutConflict(e, shortcutId)'); + expect(settings).toContain("return 'Switch to Tab N'"); + }); }); diff --git a/test/terminal-copy-selection.test.ts b/test/terminal-copy-selection.test.ts index 4770ad0b..a3974482 100644 --- a/test/terminal-copy-selection.test.ts +++ b/test/terminal-copy-selection.test.ts @@ -138,8 +138,8 @@ describe('smart-copy wiring invariants', () => { // 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') + APP_SOURCE.indexOf('_shortcutActions() {'), + APP_SOURCE.indexOf('isUserBoundShortcutEvent(e) {') ); expect(actionsBlock.length).toBeGreaterThan(0); expect(actionsBlock).not.toContain('copyTerminalSelection'); From b7be8db80b1e098d6afac3a37600d98f5f16fd28 Mon Sep 17 00:00:00 2001 From: Devvyn <22340871+opticon454@users.noreply.github.com> Date: Sat, 10 Oct 2026 14:27:54 +0800 Subject: [PATCH 02/50] fix(deepseek): find pnpm when installing the terminal profile, and install it with dsh --- .changeset/deepseek-install-pnpm-path.md | 5 +++++ config/clis.stock.json | 4 ++-- src/config/cli-registry/stock.ts | 6 ++++-- src/utils/deepseek-cli-resolver.ts | 2 +- src/web/public/session-ui.js | 2 +- src/web/routes/system-routes.ts | 7 +++++-- test/deepseek-install-profile-path.test.ts | 18 ++++++++++++++++++ 7 files changed, 36 insertions(+), 8 deletions(-) create mode 100644 .changeset/deepseek-install-pnpm-path.md create mode 100644 test/deepseek-install-profile-path.test.ts diff --git a/.changeset/deepseek-install-pnpm-path.md b/.changeset/deepseek-install-pnpm-path.md new file mode 100644 index 00000000..412d1264 --- /dev/null +++ b/.changeset/deepseek-install-pnpm-path.md @@ -0,0 +1,5 @@ +--- +'aicodeman': patch +--- + +Installing DeepSeek from CLI management no longer leaves it "not installed". The `dsh` terminal profile install ran `dsh plugin`, which needs `pnpm` on its PATH, but the server's own PATH usually lacks the user prefix (`~/.local/bin`) where CLI management puts `dsh`. The profile install now puts `dsh`'s directory first on PATH, and the DeepSeek install command installs `pnpm` alongside `dsh`. diff --git a/config/clis.stock.json b/config/clis.stock.json index 5a86af0e..d1d3e609 100644 --- a/config/clis.stock.json +++ b/config/clis.stock.json @@ -238,8 +238,8 @@ }, "install": { "command": { - "linux": "npm install -g @deepseek-ai/dsh", - "darwin": "npm install -g @deepseek-ai/dsh" + "linux": "npm install -g @deepseek-ai/dsh pnpm", + "darwin": "npm install -g @deepseek-ai/dsh pnpm" }, "npmPackage": "@deepseek-ai/dsh", "docsUrl": "https://github.com/deepseek-ai/deepseek-harness", diff --git a/src/config/cli-registry/stock.ts b/src/config/cli-registry/stock.ts index ae497207..6a0b0af6 100644 --- a/src/config/cli-registry/stock.ts +++ b/src/config/cli-registry/stock.ts @@ -1259,8 +1259,10 @@ const DEEPSEEK: CliEntry = { launcherTargetParam: 'profile', install: { command: { - linux: 'npm install -g @deepseek-ai/dsh', - darwin: 'npm install -g @deepseek-ai/dsh', + // pnpm rides along: `dsh plugin` spawns a literal `pnpm` with no npm fallback, so the + // terminal profile this CLI needs before it can run cannot be installed without it (#352). + linux: 'npm install -g @deepseek-ai/dsh pnpm', + darwin: 'npm install -g @deepseek-ai/dsh pnpm', }, npmPackage: '@deepseek-ai/dsh', docsUrl: 'https://github.com/deepseek-ai/deepseek-harness', diff --git a/src/utils/deepseek-cli-resolver.ts b/src/utils/deepseek-cli-resolver.ts index b815a761..81a69584 100644 --- a/src/utils/deepseek-cli-resolver.ts +++ b/src/utils/deepseek-cli-resolver.ts @@ -84,7 +84,7 @@ export const DEEPSEEK_VERSION_REGEX = /(?:^|\s)v?(\d+\.\d+\.\d+(?:-[0-9A-Za-z][0 */ export const DEEPSEEK_IDENTITY_REGEX = /DeepSeek\s+Harness/i; -const DEEPSEEK_NOT_FOUND = 'DeepSeek Harness CLI (dsh) not found. Install with: npm install -g @deepseek-ai/dsh'; +const DEEPSEEK_NOT_FOUND = 'DeepSeek Harness CLI (dsh) not found. Install with: npm install -g @deepseek-ai/dsh pnpm'; /** Where profiles live: `$DSH_HOME/profiles`, defaulting to `~/.dsh/profiles`. */ export function resolveDshHome(): string { diff --git a/src/web/public/session-ui.js b/src/web/public/session-ui.js index 3ec17fe0..91771570 100644 --- a/src/web/public/session-ui.js +++ b/src/web/public/session-ui.js @@ -92,7 +92,7 @@ const RUN_MODE_LAUNCH = { }, deepseek: { label: 'DeepSeek', - installHint: 'DeepSeek Harness CLI (dsh) not found. Install with: npm install -g @deepseek-ai/dsh', + installHint: 'DeepSeek Harness CLI (dsh) not found. Install with: npm install -g @deepseek-ai/dsh pnpm', // The two-part availability check is deliberate. `dsh` being installed is // not enough — DeepSeek ships no terminal front door, so a box can have a // perfect binary and nothing a pane can run. diff --git a/src/web/routes/system-routes.ts b/src/web/routes/system-routes.ts index e2251c24..91f49b04 100644 --- a/src/web/routes/system-routes.ts +++ b/src/web/routes/system-routes.ts @@ -617,13 +617,16 @@ export function registerSystemRoutes( // synchronous-spawn precedent this endpoint is modelled on. detached: true, // Inherit the environment: this needs a HOME to resolve $DSH_HOME - // against, and a PATH carrying `pnpm`. ⚠️ `dsh plugin` does NOT bundle a + // against, and a PATH carrying `pnpm` (see the PATH below). ⚠️ `dsh plugin` does NOT bundle a // package manager — it `spawnSync`s a literal `pnpm` with no npm // fallback, so on a host without one this exits 127 and dsh's own // stderr ("pnpm not found on PATH") is what reaches the caller through // the OPERATION_FAILED detail below. That is the same missing // dependency that broke the docker agent image in issue #352. - env: process.env, + // PATH leads with the directory `dsh` itself resolved to: CLI management installs + // `dsh` and `pnpm` into the user prefix (`~/.local/bin`), which a service's own PATH + // usually lacks, so `dsh plugin` could not find the `pnpm` sitting next to it. + env: { ...process.env, PATH: [dir, process.env.PATH].filter(Boolean).join(':') }, }); } catch (err) { resolve({ code: null, output: `spawn failed: ${getErrorMessage(err)}`, timedOut: false }); diff --git a/test/deepseek-install-profile-path.test.ts b/test/deepseek-install-profile-path.test.ts new file mode 100644 index 00000000..db88264d --- /dev/null +++ b/test/deepseek-install-profile-path.test.ts @@ -0,0 +1,18 @@ +import { describe, it, expect } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import { STOCK_CLIS } from '../src/config/cli-registry/stock.js'; + +const root = resolve(import.meta.dirname, '..'); + +describe('DeepSeek install', () => { + it('installs pnpm with dsh, since `dsh plugin` spawns a literal pnpm', () => { + const ds = STOCK_CLIS.find((c) => (c.id as string) === 'deepseek')!; + expect(ds.discovery.install.command?.linux).toBe('npm install -g @deepseek-ai/dsh pnpm'); + }); + + it("puts dsh's own directory first on the profile install's PATH", () => { + const src = readFileSync(resolve(root, 'src/web/routes/system-routes.ts'), 'utf8'); + expect(src).toMatch(/env: \{ \.\.\.process\.env, PATH: \[dir, process\.env\.PATH\]/); + }); +}); From 3776bbd43139bb180a978f8ed2df91b8d06a24df Mon Sep 17 00:00:00 2001 From: Devvyn <22340871+opticon454@users.noreply.github.com> Date: Wed, 14 Oct 2026 02:47:58 +0800 Subject: [PATCH 03/50] fix(shortcuts): address review on rebinding (gate holes, re-enable/Reset duplicates, labels, tests) - isUserBoundShortcutEvent skips toggleSessionSidebar (it has its own gate) and an entry re-captured onto its own default, so ^L still reaches the CLI. - Re-enabling a shortcut and Reset run the duplicate check and refuse when the chord was taken meanwhile; Ctrl/Shift+Enter and Ctrl/Cmd+V are reserved with Alt+1-9, and the Alt-digit check no longer excludes Cmd. - Alt+[ / Alt+] keep their Alt/Option+[ labels via displayBindings. - Behavioural tests (vm harness) replace the source-string test. - Docs: wiki Rebinding / Why-chords-behave-oddly, architecture-invariants note. - Small: _shortcutActions indentation, stale Alt-block comment, typeof check instead of Object.hasOwn, full-width parentheses in zh-CN. --- .changeset/keybinding-polish.md | 2 +- docs/architecture-invariants.md | 2 +- docs/wiki/Keyboard-Shortcuts.md | 14 +- src/web/public/app.js | 60 ++++---- src/web/public/i18n.js | 2 +- src/web/public/settings-ui.js | 54 +++++++- test/keyboard-shortcuts.test.ts | 10 -- test/shortcut-rebinding.test.ts | 235 ++++++++++++++++++++++++++++++++ 8 files changed, 337 insertions(+), 42 deletions(-) create mode 100644 test/shortcut-rebinding.test.ts diff --git a/.changeset/keybinding-polish.md b/.changeset/keybinding-polish.md index 2ca2e0b0..cdba12e3 100644 --- a/.changeset/keybinding-polish.md +++ b/.changeset/keybinding-polish.md @@ -2,4 +2,4 @@ 'aicodeman': minor --- -Shortcut rebinding (App Settings → Shortcuts) now covers Previous Session and the alternate Next Session, which were fixed at Alt/Option+[ and Alt/Option+]. Capturing a chord another action already owns is refused with a message naming that action (Alt+1-9 are reserved for tab switching). A chord you rebound to an app action no longer also types into the terminal: a rebind to Alt+J used to switch tabs and send ESC j to the session as well. +Shortcut rebinding (App Settings → Shortcuts) now covers Previous Session and the alternate Next Session, which were fixed at Alt/Option+[ and Alt/Option+]. Capturing a chord another action already owns is refused with a message naming that action (Alt+1-9 are reserved for tab switching, Ctrl/Shift+Enter for newline and Ctrl/Cmd+V for paste); re-enabling a disabled shortcut and Reset run the same check. A chord you rebound to an app action no longer also types into the terminal: a rebind to Alt+J used to switch tabs and send ESC j to the session as well. diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index ce51923b..ec53b87e 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -672,7 +672,7 @@ Matching semantics: a query containing `/` matches the relative path, otherwise ### Command palette and shortcut registry -**Command palette + shortcut registry** (COD-151/153/157/192, #146): `Ctrl/Cmd/Alt+K` opens the session palette (fuzzy search over live sessions; "Browse all sessions" → the Session Manager modal backed by `GET /api/sessions/unified`); the quick-start case `` is fronted by a searchable picker (`buildCasePickerOptions`/`formatCasePickerLabel` — remote cases render `name @ hostId`). Shortcuts live in a rebindable registry (`DEFAULT_SHORTCUTS`/`getShortcutRegistry()`/`matchesShortcutEvent()` in app.js; overrides persist under `settings.shortcutOverrides` via `saveAppSettingsToStorage`); App Settings → Shortcuts renders capture/disable rows; `Ctrl+?` opens the registry-driven overlay (footer links to the full `#helpModal` reference). ⚠️ Palette-chord keys must ALSO be swallowed in `attachCustomKeyEventHandler` (terminal-ui.js) or xterm writes the control byte (0x0B) into the PTY. ⚠️ The global Escape handler (app.js) calls every overlay close method on every Escape, in the capture phase and so before the focused element's own Escape handler, which means a close method with side effects beyond hiding must return early when its overlay is not open: `closeCommandPalette()` and `closeSessionManager()` hand focus back on close (`_restoreOverlayFocus()`, panels-ui.js), and run unconditionally that restore stole focus from split Pane B and from any text field, and turned the inline tab rename's Escape into a commit. ⚠️ `saveAppSettings()` rebuilds settings from the DOM — keys edited elsewhere (`shortcutOverrides`, `showTokenCount`, `showCost`) need explicit `_prev` carry-over. ⚠️ Every xterm key handler (Pane A in terminal-ui.js, Pane B in terminal-tile.js) must call `isUserBoundShortcutEvent(ev)`, which is true only for a chord the user REBOUND a generically dispatched action to: the document capture handler's `preventDefault()` does not stop xterm, so a rebind to e.g. Alt+J would otherwise also type `ESC j` into the PTY. Defaults are deliberately not gated (Ctrl+L must reach the CLI for its redraw), so the gate also skips an entry re-captured onto its own default, `toggleSessionSidebar` (it has its own gate, `shouldToggleSessionSidebarFromShortcut`) and copy-selection. Capturing, re-enabling and Reset all go through `findShortcutConflict`/`findShortcutBindingConflict` (settings-ui.js); Alt+1-9, Ctrl/Shift+Enter and Ctrl/Cmd+V are reserved there. **A key gate on a chord that can carry Shift alone must swallow the KEYPRESS too** (#520). xterm calls the custom handler from `attachCustomKeyEventHandler` for `keydown`, `keypress` AND `keyup`, and its own `_keyPress` drops a keypress carrying Ctrl, Alt or Meta but not one whose only modifier is Shift. So a gate that returns `false` for `ev.type === 'keydown'` alone is safe for a Ctrl or Alt chord (the palette's Ctrl+K, Alt+B, Ctrl+V, Ctrl+Z and the keydown-only smart copy below) and leaks for a Shift-only one. That is how Shift+Enter submitted the prompt from April to #520: its keypress fell through to `onData` as a bare `\r`, which goes out synchronously over the WebSocket ahead of the async `send-key` fetch, so the CR reached the PTY before the newline did. Ctrl+Enter worked all along only because Chrome fires no keypress for it. ⚠️ The Shift/Ctrl+Enter gate in terminal-ui.js and its twin in Pane B (terminal-split.js) return `false` for EVERY event type and key only the `send-key` POST on `keydown`; never narrow either back to keydown, and give any new Shift-only chord the same shape. Swallowing the keyup as well is harmless (xterm only skips a focus and cursor-style refresh for that one event). Tests: `test/shift-enter-keypress-swallowed.test.ts` (CI gate, a source guard on both files) and `test/shift-enter-keypress.browser.test.ts` (browser suite, which the gate does not run: real keys against both real handlers in the real app). Settings → Terminal & Input → **Key tester** shows what a device reports for each of the three events. diff --git a/docs/wiki/Keyboard-Shortcuts.md b/docs/wiki/Keyboard-Shortcuts.md index 3996685c..768085c1 100644 --- a/docs/wiki/Keyboard-Shortcuts.md +++ b/docs/wiki/Keyboard-Shortcuts.md @@ -67,6 +67,13 @@ default. See [Tile Grid](Tile-Grid). **App Settings → Shortcuts.** Bindings live in a registry with per-user overrides, so a rebind is stored as an override on top of the default rather than replacing the table. +Previous Session (`Alt+[`) and Next Session (alternate, `Alt+]`) are rows like any other, +so they can be rebound too. + +A chord another action already uses is refused, and the message names that action. The same +check runs when you re-enable a disabled shortcut and when you press Reset: if its chord was +given to something else in the meantime, it stays as it was. `Alt+1` to `Alt+9`, +`Shift+Enter`/`Ctrl+Enter` and `Ctrl+V`/`Cmd+V` are reserved. Two things are deliberately not rebindable: @@ -79,8 +86,11 @@ Two things are deliberately not rebindable: The terminal sees keystrokes before the app does. Any chord the app claims has to also be swallowed at the terminal layer, or xterm writes the control byte into the session as well -as triggering the action. If you rebind something to a chord the terminal cares about -(`Ctrl+D`, say), expect the CLI to see it too. +as triggering the action. A chord **you** rebind an action to is kept out of the terminal +too, so rebinding Previous Session to `Alt+J` switches the session and sends nothing to the +CLI. The built-in defaults are deliberately not held back this way: `Ctrl+L` clears the +screen and also reaches the CLI, which redraws on it. Binding an action back onto its own +default behaves like the default. `Alt+1` through `Alt+9` are matched on **physical key position** rather than the character produced, so macOS Option layouts that produce `¡™£` still switch tabs. diff --git a/src/web/public/app.js b/src/web/public/app.js index 0cd59388..5e4fb009 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -532,8 +532,8 @@ const DEFAULT_SHORTCUTS = [ group: 'Session', label: 'Toggle Session Sidebar', // Alt+B, not Ctrl+B: Ctrl+B must reach the terminal (tmux prefix, - // readline backward-char). The Alt block below claims only Digit1-9 and - // the brackets, and the registry claims Alt for KeyK and Slash only. + // readline backward-char). The Alt block below claims only Digit1-9; the + // brackets are registry entries, and the registry claims Alt for KeyK and Slash only. bindings: [{ modifiers: ['alt'], key: 'b', code: 'KeyB' }], action: 'toggleSessionSidebar', }, @@ -642,6 +642,7 @@ const DEFAULT_SHORTCUTS = [ id: 'previous-session', group: 'Session', label: 'Previous Session', + displayBindings: ['Alt/Option+['], bindings: [{ modifiers: ['alt'], code: 'BracketLeft' }], action: 'previousSession', }, @@ -649,6 +650,7 @@ const DEFAULT_SHORTCUTS = [ id: 'next-session-alt', group: 'Session', label: 'Next Session (alternate)', + displayBindings: ['Alt/Option+]'], bindings: [{ modifiers: ['alt'], code: 'BracketRight' }], action: 'nextSession', }, @@ -1450,7 +1452,8 @@ class CodemanApp { // Option/Alt session navigation uses physical key CODES, not e.key, so macOS // keyboard layouts that emit special characters under Option (Option+1 -> ¡, // Option+[ -> "“") still switch sessions. e.code is the physical key regardless - // of layout. Option+1-9 = switch by index. Option+[ / ] (prev / next) live in the shortcut registry, so they can be rebound. + // of layout. Option+1-9 = switch by index. Option+[ / ] (prev / next) live in the + // shortcut registry, so they can be rebound. if (e.altKey && !e.ctrlKey && !e.shiftKey) { const code = e.code || ''; const digitMatch = code.match(/^Digit([1-9])$/); @@ -10196,39 +10199,46 @@ class CodemanApp { * absent here (copy selection, the palette, the tile chords) have their own dispatch. */ _shortcutActions() { - return { - showShortcutOverlay: () => this.showShortcutOverlay(), - killActiveSession: () => this.killActiveSession(), - nextSession: () => this.nextSession(), - previousSession: () => this.prevSession(), - clearTerminal: () => this.clearTerminal(), - restoreTerminalSize: () => this.restoreTerminalSize(), - increaseFontSize: () => this.increaseFontSize(), - decreaseFontSize: () => this.decreaseFontSize(), - toggleVoiceInput: () => VoiceInput.toggle(), - moveActiveTabLeft: () => this.moveActiveTabLeft(), - moveActiveTabRight: () => this.moveActiveTabRight(), - toggleSessionSidebar: () => this.toggleSessionSidebar(), - }; + return { + showShortcutOverlay: () => this.showShortcutOverlay(), + killActiveSession: () => this.killActiveSession(), + nextSession: () => this.nextSession(), + previousSession: () => this.prevSession(), + clearTerminal: () => this.clearTerminal(), + restoreTerminalSize: () => this.restoreTerminalSize(), + increaseFontSize: () => this.increaseFontSize(), + decreaseFontSize: () => this.decreaseFontSize(), + toggleVoiceInput: () => VoiceInput.toggle(), + moveActiveTabLeft: () => this.moveActiveTabLeft(), + moveActiveTabRight: () => this.moveActiveTabRight(), + toggleSessionSidebar: () => this.toggleSessionSidebar(), + }; } /** * True when `e` is a chord the USER rebound to a generically dispatched action. * The document handler has already run the action, but its preventDefault() does not * stop xterm, so without this gate a rebind to e.g. Alt+J would also type ESC j into the - * session. Defaults are not covered on purpose: several (Ctrl+L) rely on reaching the PTY. + * session. Defaults are not covered on purpose: several (Ctrl+L) rely on reaching the PTY, + * and neither is a chord that matches the entry's own default (a re-capture of it). */ isUserBoundShortcutEvent(e) { if (!e.ctrlKey && !e.metaKey && !e.altKey) return false; const overrides = this.loadAppSettingsFromStorage().shortcutOverrides || {}; const actions = this._shortcutActions(); - return this.getShortcutRegistry().some( - (shortcut) => - !shortcut.disabled && - Array.isArray(overrides[shortcut.id]?.bindings) && - Object.hasOwn(actions, shortcut.action) && - this.matchesShortcutEvent(e, shortcut) - ); + return this.getShortcutRegistry().some((shortcut) => { + if (shortcut.disabled || !Array.isArray(overrides[shortcut.id]?.bindings)) return false; + // toggleSessionSidebar has its own gate (shouldToggleSessionSidebarFromShortcut) that + // lets the chord through when the sidebar layout is off; swallowing it here would make a + // rebound sidebar key dead in the header layout. + if (shortcut.action === 'toggleSessionSidebar') return false; + if (typeof actions[shortcut.action] !== 'function') return false; + if (!this.matchesShortcutEvent(e, shortcut)) return false; + // Re-capturing the entry's own default (Ctrl+L for Clear Terminal) must keep reaching the + // CLI: clearTerminal() relies on ^L for the redraw. + const fallback = DEFAULT_SHORTCUTS.find((d) => d.id === shortcut.id); + return !this.matchesShortcutEvent(e, fallback); + }); } // Returns the merged shortcut list: DEFAULT_SHORTCUTS with any per-shortcut diff --git a/src/web/public/i18n.js b/src/web/public/i18n.js index ff997009..37777730 100644 --- a/src/web/public/i18n.js +++ b/src/web/public/i18n.js @@ -1141,7 +1141,7 @@ 'Previous / Next Session': '上一个 / 下一个会话', 'Next Session': '下一个会话', 'Previous Session': '上一个会话', - 'Next Session (alternate)': '下一个会话(备用)', + 'Next Session (alternate)': '下一个会话（备用）', 'Switch to Tab N': '切换到第 N 个标签页', 'Move Active Tab Left': '向左移动当前标签页', 'Move Active Tab Right': '向右移动当前标签页', diff --git a/src/web/public/settings-ui.js b/src/web/public/settings-ui.js index d1d43503..7cd79406 100644 --- a/src/web/public/settings-ui.js +++ b/src/web/public/settings-ui.js @@ -4730,10 +4730,16 @@ Object.assign(CodemanApp.prototype, { /** Label of the action that already owns the chord in `e`, or '' when it is free. */ findShortcutConflict(e, shortcutId) { - // Alt+1-9 switch tabs by index; they are fixed, not in the registry. - if (e.altKey && !e.ctrlKey && !e.metaKey && !e.shiftKey && /^Digit[1-9]$/.test(e.code || '')) { + // Chords the keydown handlers claim before (or outside) the registry: they are fixed, so a + // rebind onto one of them would be dead or would hijack it. Alt+1-9 switch tabs by index + // (the document handler does not exclude Cmd); Ctrl/Shift+Enter insert a newline and + // Ctrl/Cmd+V pastes, and the user-bound gate sits above those branches in the xterm handler. + if (e.altKey && !e.ctrlKey && !e.shiftKey && /^Digit[1-9]$/.test(e.code || '')) { return 'Switch to Tab N'; } + // (Alt+Shift+Enter is the tile grid's Zoom chord, a registry entry, so Alt stays free.) + if (e.key === 'Enter' && !e.altKey && (e.ctrlKey || e.shiftKey)) return 'Insert Newline'; + if ((e.ctrlKey || e.metaKey) && !e.altKey && !e.shiftKey && String(e.key).toLowerCase() === 'v') return 'Paste'; if (typeof this.getShortcutRegistry !== 'function' || typeof this.matchesShortcutEvent !== 'function') return ''; const other = this.getShortcutRegistry().find( (shortcut) => @@ -4742,6 +4748,30 @@ Object.assign(CodemanApp.prototype, { return other ? other.label : ''; }, + /** + * Label of the action that already owns a chord in `shortcut`'s bindings, or ''. Used when a + * binding comes back without a capture (re-enable, Reset), where the chord may have been + * given to another action in the meantime. + */ + findShortcutBindingConflict(shortcut) { + for (const binding of shortcut?.bindings || []) { + const mods = binding.modifiers || []; + const taken = this.findShortcutConflict( + { + ctrlKey: mods.includes('ctrl'), + metaKey: mods.includes('meta'), + shiftKey: mods.includes('shift'), + altKey: mods.includes('alt'), + key: binding.key || '', + code: binding.code || '', + }, + shortcut.id + ); + if (taken) return taken; + } + return ''; + }, + describeShortcutEvent(e) { const parts = []; if (e.ctrlKey) parts.push('Ctrl'); @@ -4753,6 +4783,15 @@ Object.assign(CodemanApp.prototype, { }, resetShortcutOverride(shortcutId) { + // Reset puts the default chord back (and re-enables the row): refuse when another action + // has taken that chord since, instead of leaving two actions on one key. + const fallback = typeof DEFAULT_SHORTCUTS !== 'undefined' ? DEFAULT_SHORTCUTS.find((d) => d.id === shortcutId) : null; + const taken = fallback ? this.findShortcutBindingConflict(fallback) : ''; + if (taken) { + this.renderShortcutSettingsList(); + this.showToast?.(`The default chord is already used by ${taken}`, 'error'); + return; + } const settings = this.loadAppSettingsFromStorage(); const shortcutOverrides = { ...(settings.shortcutOverrides || {}) }; delete shortcutOverrides[shortcutId]; @@ -4762,6 +4801,17 @@ Object.assign(CodemanApp.prototype, { }, toggleShortcutEnabled(shortcutId, enabled) { + if (enabled) { + // Re-enabling brings the row's chord back into play: leave it disabled when another action + // has been bound to that chord while it was off. + const shortcut = this.getShortcutRegistry?.().find((s) => s.id === shortcutId); + const taken = this.findShortcutBindingConflict(shortcut); + if (taken) { + this.renderShortcutSettingsList(); + this.showToast?.(`Its chord is already used by ${taken}`, 'error'); + return; + } + } const settings = this.loadAppSettingsFromStorage(); const shortcutOverrides = { ...(settings.shortcutOverrides || {}) }; shortcutOverrides[shortcutId] = { ...(shortcutOverrides[shortcutId] || {}), disabled: !enabled }; diff --git a/test/keyboard-shortcuts.test.ts b/test/keyboard-shortcuts.test.ts index 288d9885..b46c69b6 100644 --- a/test/keyboard-shortcuts.test.ts +++ b/test/keyboard-shortcuts.test.ts @@ -85,14 +85,4 @@ describe('keyboard shortcuts', () => { expect(readme).toContain('`Ctrl/Cmd+C`'); expect(readme).toContain('`Ctrl+Shift+C`'); }); - - it('keeps a user-rebound chord out of the PTY and refuses duplicate bindings', () => { - const tile = readFileSync('src/web/public/terminal-tile.js', 'utf8'); - const settings = readFileSync('src/web/public/settings-ui.js', 'utf8'); - expect(appSource).toContain('isUserBoundShortcutEvent(e) {'); - expect(terminalUiSource).toContain('this.isUserBoundShortcutEvent?.(ev)) return false'); - expect(tile).toContain('global.app?.isUserBoundShortcutEvent?.(ev)) return false'); - expect(settings).toContain('this.findShortcutConflict(e, shortcutId)'); - expect(settings).toContain("return 'Switch to Tab N'"); - }); }); diff --git a/test/shortcut-rebinding.test.ts b/test/shortcut-rebinding.test.ts new file mode 100644 index 00000000..a9485c2e --- /dev/null +++ b/test/shortcut-rebinding.test.ts @@ -0,0 +1,235 @@ +/** + * @fileoverview Behaviour of shortcut rebinding: Alt+[ / Alt+] as registry entries, the + * user-bound gate that keeps a rebound chord out of the PTY, and refusal of duplicate chords. + * + * Real code under test: constants.js + app.js + settings-ui.js in one `vm` context (the + * makeApp() harness of ctrl-w-never-closes.test.ts, with settings-ui.js loaded after app.js). + */ +import { readFileSync } from 'node:fs'; +import { performance } from 'node:perf_hooks'; +import { resolve } from 'node:path'; +import vm from 'node:vm'; +import { describe, expect, it, vi } from 'vitest'; + +const read = (f: string) => readFileSync(resolve(import.meta.dirname, `../src/web/public/${f}`), 'utf8'); + +type Shortcut = { id: string; label: string; action?: string; bindings?: unknown[]; disabled?: boolean }; +type App = { + getShortcutRegistry(): Shortcut[]; + matchesShortcutEvent(e: Record, s: Shortcut): boolean; + isUserBoundShortcutEvent(e: Record): boolean; + findShortcutConflict(e: Record, id: string): string; + onShortcutCaptureKeydown(e: Record, id: string): void; + toggleShortcutEnabled(id: string, enabled: boolean): void; + resetShortcutOverride(id: string): void; + loadAppSettingsFromStorage: () => Record; + saveAppSettingsToStorage: (s: Record) => void; + renderShortcutSettingsList: () => void; + showToast: (msg: string, type?: string) => void; +}; + +function makeApp(initial: Record = {}) { + const settings: Record = structuredClone(initial); + const context = vm.createContext({ + console, + performance, + setInterval: vi.fn(), + clearInterval: vi.fn(), + setTimeout, + clearTimeout, + requestAnimationFrame: vi.fn(), + HTMLCanvasElement: class HTMLCanvasElement {}, + WebSocket: { OPEN: 1 }, + fetch: vi.fn(), + document: { addEventListener: vi.fn(), getElementById: () => null, querySelector: () => null }, + localStorage: { length: 0, key: vi.fn(), getItem: vi.fn(), setItem: vi.fn(), removeItem: vi.fn() }, + window: { addEventListener: vi.fn(), removeEventListener: vi.fn() }, + MobileDetection: { getDeviceType: () => 'desktop', isMobile: () => false, isTouchDevice: () => false }, + }); + vm.runInContext(`${read('constants.js')}\n${read('app.js')}\nglobalThis.__CodemanApp = CodemanApp;`, context); + vm.runInContext(read('settings-ui.js'), context, { filename: 'settings-ui.js' }); + const CodemanApp = (context as unknown as { __CodemanApp: { prototype: object } }).__CodemanApp; + const app = Object.create(CodemanApp.prototype) as App; + app.loadAppSettingsFromStorage = () => settings; + app.saveAppSettingsToStorage = (s) => { + const next = { ...s }; + Object.keys(settings).forEach((k) => delete settings[k]); + Object.assign(settings, next); + }; + app.renderShortcutSettingsList = vi.fn(); + app.showToast = vi.fn(); + return { app, settings }; +} + +const key = (o: Record) => ({ + type: 'keydown', + key: '', + code: '', + ctrlKey: false, + metaKey: false, + shiftKey: false, + altKey: false, + preventDefault: vi.fn(), + stopPropagation: vi.fn(), + ...o, +}); + +const ownerOf = (app: App, e: Record) => + app.getShortcutRegistry().find((s) => !s.disabled && s.action && app.matchesShortcutEvent(e, s))?.action; + +describe('Alt+[ and Alt+] are registry entries', () => { + it('resolve to previousSession and nextSession by physical key', () => { + const { app } = makeApp(); + expect(ownerOf(app, key({ altKey: true, key: '[', code: 'BracketLeft' }))).toBe('previousSession'); + expect(ownerOf(app, key({ altKey: true, key: ']', code: 'BracketRight' }))).toBe('nextSession'); + }); + + it('match the macOS Option layout that prints a curly quote for Option+[', () => { + const { app } = makeApp(); + expect(ownerOf(app, key({ altKey: true, key: '“', code: 'BracketLeft' }))).toBe('previousSession'); + }); + + it('keep their Alt/Option+[ labels until rebound', () => { + const { app } = makeApp({ + shortcutOverrides: { 'next-session-alt': { bindings: [{ modifiers: ['alt'], key: 'j' }] } }, + }); + const byId = (id: string) => + app.getShortcutRegistry().find((s) => s.id === id) as Shortcut & { displayBindings?: string[] }; + expect(byId('previous-session').displayBindings).toEqual(['Alt/Option+[']); + expect(byId('next-session-alt').displayBindings).toBeUndefined(); + }); +}); + +describe('isUserBoundShortcutEvent', () => { + const altJ = key({ altKey: true, key: 'j', code: 'KeyJ' }); + + it('is false with no overrides, even for a default chord', () => { + const { app } = makeApp(); + expect(app.isUserBoundShortcutEvent(key({ altKey: true, key: '[', code: 'BracketLeft' }))).toBe(false); + expect(app.isUserBoundShortcutEvent(key({ ctrlKey: true, key: 'l', code: 'KeyL' }))).toBe(false); + }); + + it('is true for a chord the user rebound an action to', () => { + const { app } = makeApp({ + shortcutOverrides: { 'previous-session': { bindings: [{ modifiers: ['alt'], key: 'j', code: 'KeyJ' }] } }, + }); + expect(app.isUserBoundShortcutEvent(altJ)).toBe(true); + }); + + it('is false for the copy-selection rebind, which has its own dispatch', () => { + const { app } = makeApp({ + shortcutOverrides: { 'copy-selection': { bindings: [{ modifiers: ['alt'], key: 'j', code: 'KeyJ' }] } }, + }); + expect(app.isUserBoundShortcutEvent(altJ)).toBe(false); + }); + + it('is false for a rebound sidebar toggle, whose own gate lets the chord through', () => { + const { app } = makeApp({ + shortcutOverrides: { 'toggle-session-sidebar': { bindings: [{ modifiers: ['alt'], key: 's', code: 'KeyS' }] } }, + }); + expect(app.isUserBoundShortcutEvent(key({ altKey: true, key: 's', code: 'KeyS' }))).toBe(false); + }); + + it('is false when the entry was re-captured onto its own default (Ctrl+L still reaches the CLI)', () => { + const { app } = makeApp({ + shortcutOverrides: { 'clear-terminal': { bindings: [{ modifiers: ['ctrl'], key: 'l', code: 'KeyL' }] } }, + }); + expect(app.isUserBoundShortcutEvent(key({ ctrlKey: true, key: 'l', code: 'KeyL' }))).toBe(false); + }); + + it('is false for a disabled rebound entry', () => { + const { app } = makeApp({ + shortcutOverrides: { + 'previous-session': { bindings: [{ modifiers: ['alt'], key: 'j', code: 'KeyJ' }], disabled: true }, + }, + }); + expect(app.isUserBoundShortcutEvent(altJ)).toBe(false); + }); +}); + +describe('duplicate chords are refused', () => { + const ctrlL = key({ ctrlKey: true, key: 'l', code: 'KeyL' }); + + it("a capture onto another action's chord names the owner and stores nothing", () => { + const { app, settings } = makeApp(); + app.onShortcutCaptureKeydown(ctrlL, 'next-session-alt'); + expect(app.showToast).toHaveBeenCalledWith(expect.stringContaining('already used by Clear Terminal'), 'error'); + expect(settings.shortcutOverrides).toBeUndefined(); + }); + + it("a shortcut's own id and a disabled owner are free", () => { + const { app } = makeApp({ shortcutOverrides: { 'previous-session': { disabled: true } } }); + expect(app.findShortcutConflict(ctrlL, 'clear-terminal')).toBe(''); + expect(app.findShortcutConflict(key({ altKey: true, key: '[', code: 'BracketLeft' }), 'next-session-alt')).toBe(''); + }); + + it.each([ + ['Alt+1', { altKey: true, key: '¡', code: 'Digit1' }], + ['Cmd+Alt+1', { altKey: true, metaKey: true, key: '¡', code: 'Digit1' }], + ['Ctrl+Enter', { ctrlKey: true, key: 'Enter', code: 'Enter' }], + ['Shift+Enter', { shiftKey: true, key: 'Enter', code: 'Enter' }], + ['Ctrl+V', { ctrlKey: true, key: 'v', code: 'KeyV' }], + ['Cmd+V', { metaKey: true, key: 'v', code: 'KeyV' }], + ])('%s is reserved', (_n, ev) => { + const { app } = makeApp(); + expect(app.findShortcutConflict(key(ev), 'previous-session')).not.toBe(''); + }); + + it('no two default shortcuts share a chord', () => { + const { app } = makeApp(); + for (const s of app.getShortcutRegistry()) { + for (const b of (s.bindings || []) as { modifiers?: string[]; key?: string; code?: string }[]) { + const m = b.modifiers || []; + const ev = key({ + ctrlKey: m.includes('ctrl'), + metaKey: m.includes('meta'), + shiftKey: m.includes('shift'), + altKey: m.includes('alt'), + key: b.key || '', + code: b.code || '', + }); + expect({ id: s.id, owner: app.findShortcutConflict(ev, s.id) }).toEqual({ id: s.id, owner: '' }); + } + } + }); + + it('re-enabling a disabled action whose chord was taken is refused and leaves it disabled', () => { + const { app, settings } = makeApp({ + shortcutOverrides: { + 'clear-terminal': { disabled: true }, + 'next-session-alt': { bindings: [{ modifiers: ['ctrl'], key: 'l', code: 'KeyL' }] }, + }, + }); + app.toggleShortcutEnabled('clear-terminal', true); + expect(app.showToast).toHaveBeenCalledWith( + expect.stringContaining('already used by Next Session (alternate)'), + 'error' + ); + expect(settings.shortcutOverrides['clear-terminal'].disabled).toBe(true); + }); + + it('re-enabling with no clash enables it', () => { + const { app, settings } = makeApp({ shortcutOverrides: { 'clear-terminal': { disabled: true } } }); + app.toggleShortcutEnabled('clear-terminal', true); + expect(settings.shortcutOverrides['clear-terminal'].disabled).toBe(false); + }); + + it('Reset is refused while another action owns the default chord, and works once it is free', () => { + const { app, settings } = makeApp({ + shortcutOverrides: { + 'clear-terminal': { bindings: [{ modifiers: ['ctrl'], key: 'j', code: 'KeyJ' }] }, + 'next-session-alt': { bindings: [{ modifiers: ['ctrl'], key: 'l', code: 'KeyL' }] }, + }, + }); + app.resetShortcutOverride('clear-terminal'); + expect(app.showToast).toHaveBeenCalledWith( + expect.stringContaining('already used by Next Session (alternate)'), + 'error' + ); + expect(settings.shortcutOverrides['clear-terminal']).toBeDefined(); + + app.resetShortcutOverride('next-session-alt'); + app.resetShortcutOverride('clear-terminal'); + expect(settings.shortcutOverrides['clear-terminal']).toBeUndefined(); + }); +}); From 244d9a49f1b3de68644c227b3865e65ed2e9fd8c Mon Sep 17 00:00:00 2001 From: JD Date: Sat, 10 Oct 2026 12:36:34 -0400 Subject: [PATCH 04/50] feat(uploads): prompt uploads take documents The Attach button, the Compose and paste-dialog pickers, terminal drop and the clipboard now take documents beside images and videos: the office formats pdf/docx/pptx/xlsx/zip and the text family txt/md/csv/tsv/json/log/yaml/yml/xml. A document is admitted by its name's extension (the MIME a phone attaches to a .md is anyone's guess), buffered under the same 50MB image cap, and judged on its bytes by the same fail-closed sniffer: a magic-number signature for the binary formats, UTF-8 with no NUL byte for text. It is saved under a sanitized stem of its name, so Q3 report.pdf is findable as paste---Q3-report.pdf. sh/py/js/html are refused by name with a clear 400. That is a scope choice, not a security property: file-raw already serves non-media text inert, so this keeps the server from storing what it would serve or an agent could run, and html and svg are the ones deliberately kept out. One client classifier (_promptAttachKind) now returns image, video or document, and the server's dispatch mirrors it: an image MIME wins whatever the name says, a document extension takes the document branch, and a foreign extension with no image MIME is refused only after the image sniff fails, so nothing the image path accepted by its bytes is lost. --- CLAUDE.md | 2 +- README.md | 2 +- README.zh-CN.md | 2 +- docs/api-reference.md | 15 +++ docs/architecture-invariants.md | 2 +- docs/wiki/Mobile-Guide.md | 21 +++-- docs/wiki/Settings-Reference.md | 2 +- docs/wiki/Working-With-Files.md | 38 ++++++-- src/web/public/constants.js | 4 +- src/web/public/i18n.js | 6 +- src/web/public/image-input.js | 23 +++-- src/web/public/index.html | 2 +- src/web/public/keyboard-accessory.js | 28 +++--- src/web/routes/session-routes.ts | 134 ++++++++++++++++++++++----- test/link-provider-regex.test.ts | 3 + test/mobile-prompt-composer.test.ts | 38 ++++++-- test/routes/session-routes.test.ts | 131 ++++++++++++++++++++++++-- test/toolbar-attach.test.ts | 25 ++++- 18 files changed, 382 insertions(+), 96 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index adb38400..d674908b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -213,7 +213,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph **Attachments** (live external document references; all wiring in `file-routes.ts`): a **registry** maps a stable `attachmentId` to a realpath-resolved, extension-allowlisted absolute path, so browser requests never carry arbitrary absolute paths. ⚠️ The **magic-link scanner** (`codeman://attach?...` in terminal output) is **prompt-injectable**, so its scan path is force-confined to the session workspace; a hostile prompt could otherwise exfiltrate arbitrary host files over SSE. The security gate is an extension **allowlist**, not a blocklist. `document-conversion-limiter.ts` caps converter spawns globally: without it, N large docs detected at once fork N multi-minute processes, which is a resource-exhaustion vector. → [architecture-invariants#attachments](docs/architecture-invariants.md#attachments) -**Prompt uploads** (`POST /api/sessions/:id/paste-image`: clipboard paste, drop, the Compose Attach key and the phone toolbar's Attach button; client wiring in `image-input.js` and `keyboard-accessory.js`): a pick is written to `/.codeman-uploads/` and its path lands on the prompt as unsent text. The folder is IN the workspace because that is the only path identical for a local and a container agent, FLAT because `/.codeman/` would BE the data dir when the workspace is the home directory, and self-ignoring through a `.gitignore` of `*` written ONCE with `wx` (a file already there is the user's and is never overwritten; the user's own ignore file is never touched). ⚠️ The directory names live ONLY in `paste-image-gc.ts` (`UPLOAD_DIR_NAMES`): the pre-move `.claude-images/` receives nothing but stays readable for one release, so the GC sweep and the delete cleanup (through `uploadDirs()`) and the image watcher's ignore filter (reading the names) cover BOTH; retire the legacy entry there, never by hand elsewhere. ⚠️ `uploadDirs()` is async and probes the workspace BOUNDED first (`probePathKind()`, the #516 rule: an `unknown` path is skipped, never touched; the sweep keeps the stall cap and the delete passes `pastCap`), then returns only REAL directories (lstat, so a planted symlink is never one), none that is or contains `getDataDir()` (one strictly inside it only holds uploads and is listed), and none for a remote session, since both consumers delete; the check is made at list time, and the same-user race after it is accepted. Images are buffered in memory under `MAX_PASTE_IMAGE_BYTES` (50MB) after a client-side canvas re-encode. A VIDEO is never buffered: `?kind=video` streams it to disk under `MAX_PASTE_VIDEO_BYTES` (1GB, env `CODEMAN_MAX_PASTE_VIDEO_BYTES`), judges the container on the first 12 bytes as they arrive (ISO BMFF / QuickTime atom, EBML for webm, Ogg for ogv; a mislabeled body is drained unwritten from there, never parked on disk first) and unlinks on any failure, a dead transfer included. ⚠️ The flag is the ONLY way onto that path and admits video types alone (the registry's `VIDEO_ATTACHMENT_EXTENSIONS`, single-sourced: mp4/webm/mov/m4v/ogv, so an uploaded video is one the file viewer can also play; 3gp/mkv are out, since a phone's camera roll produces neither and neither previews), so an image can never ride the larger cap through memory; without it the route is byte-identical to the image-only version. Busboy takes its size limit before the part is seen, which is why the choice rides the URL and not the file. ⚠️ A video draws on its OWN per-(IP, session) rate bucket (`consumeVideoToken`, 3/min) beside the image bucket (`consumePasteToken`, 30/min): at 1GB each, 30/min would be 30GB of disk a minute, so the lower rate bounds it to 3GB; the image path keeps the 30/min bucket and is unchanged without the flag. A remote (SSH) session is refused (400) before any disk touch: its `workingDir` is the remote path, so the file would land on THIS host where the agent cannot read it (Docker is fine, the workspace is bind-mounted at the same absolute path). ⚠️ A live session's workspace can still sit on a network mount that stopped answering, so the route probes it BOUNDED (`probePathKind(session.workingDir, { pastCap: true })`, the #516 rule) before the first disk touch and refuses an `unknown` mount through `describeUnknownPath()` (OPERATION_FAILED), the way session create does, so a mount already unreachable cannot freeze a libuv worker on every upload (a mount that dies after the probe is the same residual race create accepts). ⚠️ The client classifies a pick through ONE helper, `_promptAttachKind()` (MIME type first, then the name's extension, because some Android file managers hand over a File with none); every pick and paste filter (Attach, drop, the tile grid's drop in `tile-grid.js`, the Compose and paste-dialog pickers, the clipboard handlers), the flag and the re-encode gate read it, never `file.type` directly. ⚠️ A pick that lands while the tile grid owns the terminal goes to the PTY, never the main terminal's overlay: that terminal is parked and hidden then, and a tile's Enter cannot flush it (`_tilesOwnTerminal()` in the `_attachFromToolbar` guard). Tests: `test/toolbar-attach.test.ts`, `test/mobile-prompt-composer.test.ts`, `test/paste-image-gc.test.ts`, `test/paste-image-dir-shared.test.ts`, the paste-image block of `test/routes/session-routes.test.ts`. → [architecture-invariants#buffers-uploads-and-terminal-history](docs/architecture-invariants.md#buffers-uploads-and-terminal-history) +**Prompt uploads** (`POST /api/sessions/:id/paste-image`: clipboard paste, drop, the Compose Attach key and the phone toolbar's Attach button; client wiring in `image-input.js` and `keyboard-accessory.js`): a pick is written to `/.codeman-uploads/` and its path lands on the prompt as unsent text. The folder is IN the workspace because that is the only path identical for a local and a container agent, FLAT because `/.codeman/` would BE the data dir when the workspace is the home directory, and self-ignoring through a `.gitignore` of `*` written ONCE with `wx` (a file already there is the user's and is never overwritten; the user's own ignore file is never touched). ⚠️ The directory names live ONLY in `paste-image-gc.ts` (`UPLOAD_DIR_NAMES`): the pre-move `.claude-images/` receives nothing but stays readable for one release, so the GC sweep and the delete cleanup (through `uploadDirs()`) and the image watcher's ignore filter (reading the names) cover BOTH; retire the legacy entry there, never by hand elsewhere. ⚠️ `uploadDirs()` is async and probes the workspace BOUNDED first (`probePathKind()`, the #516 rule: an `unknown` path is skipped, never touched; the sweep keeps the stall cap and the delete passes `pastCap`), then returns only REAL directories (lstat, so a planted symlink is never one), none that is or contains `getDataDir()` (one strictly inside it only holds uploads and is listed), and none for a remote session, since both consumers delete; the check is made at list time, and the same-user race after it is accepted. Images are buffered in memory under `MAX_PASTE_IMAGE_BYTES` (50MB) after a client-side canvas re-encode; a DOCUMENT (pdf, docx, pptx, xlsx, zip and the text family txt/md/csv/tsv/json/log/yaml/yml/xml; the curated `ALLOWED_DOCUMENT_EXTS`, never `SUPPORTED_ATTACHMENT_EXTENSIONS` or `EDITABLE_EXTENSIONS`, which carry sh/py/js/html) is admitted by its name's extension alone, buffered like an image, judged on its bytes by the same fail-closed `imageMagicMatchesExt` (`%PDF-`, `PK\x03\x04` for the OOXML trio and zip, UTF-8 without a NUL byte for text: a SIGNATURE or ENCODING check, never a structure or content check, so generic zip bytes pass as `.docx` and a script renamed `.txt` is text; the sh/py/js/html exclusion is a SCOPE choice by extension, keeping the server from storing what it would serve or run, not a content filter: the agent can already write any of those) and saved as `paste---` with a sanitized stem (`[A-Za-z0-9._-]`, 40 chars, a wholly non-ASCII name leaves no stem; the `paste-` prefix is what the GC matches). A VIDEO is never buffered: `?kind=video` streams it to disk under `MAX_PASTE_VIDEO_BYTES` (1GB, env `CODEMAN_MAX_PASTE_VIDEO_BYTES`), judges the container on the first 12 bytes as they arrive (ISO BMFF / QuickTime atom, EBML for webm, Ogg for ogv; a mislabeled body is drained unwritten from there, never parked on disk first) and unlinks on any failure, a dead transfer included. ⚠️ The flag is the ONLY way onto that path and admits video types alone (the registry's `VIDEO_ATTACHMENT_EXTENSIONS`, single-sourced: mp4/webm/mov/m4v/ogv, so an uploaded video is one the file viewer can also play; 3gp/mkv are out, since a phone's camera roll produces neither and neither previews), so an image can never ride the larger cap through memory; without it the route is byte-identical to the image-only version. Busboy takes its size limit before the part is seen, which is why the choice rides the URL and not the file. ⚠️ A video draws on its OWN per-(IP, session) rate bucket (`consumeVideoToken`, 3/min) beside the image bucket (`consumePasteToken`, 30/min): at 1GB each, 30/min would be 30GB of disk a minute, so the lower rate bounds it to 3GB; the image path keeps the 30/min bucket and is unchanged without the flag. A remote (SSH) session is refused (400) before any disk touch: its `workingDir` is the remote path, so the file would land on THIS host where the agent cannot read it (Docker is fine, the workspace is bind-mounted at the same absolute path). ⚠️ A live session's workspace can still sit on a network mount that stopped answering, so the route probes it BOUNDED (`probePathKind(session.workingDir, { pastCap: true })`, the #516 rule) before the first disk touch and refuses an `unknown` mount through `describeUnknownPath()` (OPERATION_FAILED), the way session create does, so a mount already unreachable cannot freeze a libuv worker on every upload (a mount that dies after the probe is the same residual race create accepts). ⚠️ Dispatch mirrors the client: an `image/*` MIME takes the IMAGE path whatever the name says (a GIF named `animation.txt` stays a GIF), a document extension takes the document branch, and anything else takes the image path exactly as before 1.36, where the HEIC sniff still recovers a HEIC under a generic name; a foreign extension with no image MIME gets its `Unsupported file type` 400 only AFTER that sniff fails, never before, so nothing the image path ever accepted by its bytes is refused. The text-mismatch log line names the failure (`nul-byte`/`not-utf8`) and never prints the bytes, since the head of a text file is user content. ⚠️ The client classifies a pick through ONE helper, `_promptAttachKind()` (MIME type first, then the name's extension, because some Android file managers hand over a File with none; returns `image`, `video` or `document`); every pick and paste filter (Attach, drop, the tile grid's drop in `tile-grid.js`, the Compose and paste-dialog pickers, the clipboard handlers), the flag and the re-encode gate read it, never `file.type` directly. ⚠️ A pick that lands while the tile grid owns the terminal goes to the PTY, never the main terminal's overlay: that terminal is parked and hidden then, and a tile's Enter cannot flush it (`_tilesOwnTerminal()` in the `_attachFromToolbar` guard). Tests: `test/toolbar-attach.test.ts`, `test/mobile-prompt-composer.test.ts`, `test/paste-image-gc.test.ts`, `test/paste-image-dir-shared.test.ts`, the paste-image block of `test/routes/session-routes.test.ts`. → [architecture-invariants#buffers-uploads-and-terminal-history](docs/architecture-invariants.md#buffers-uploads-and-terminal-history) **File-path links (terminal + chat)**: a path an agent prints is clickable on BOTH surfaces and opens the file-preview overlay. ⚠️ ONE pattern (`FILE_PATH_LINK_PATTERN` / `absoluteFilePathPattern()` in constants.js) feeds the xterm link provider AND `_linkifyFilePaths()`, a fresh instance per call (`lastIndex`). Both call `findFilePathLinks()`, which adds `~/`, relative and folder paths without overlapping a rooted match or a URL; a relative path resolves against `workingDir` (`resolveLinkedFilePath`), `~/` only server-side on the session's host (attachment route), and `terminalPathLine()` joins a path a program hard-wrapped short of the edge. ⚠️ A folder link opens the **folder grid**: `POST /api/sessions/:id/folders` guards + GRANTS the folder (`folder-listing.ts`, own registry, never the 200-capped attachment registry), files come by bare name via `…/folders/:folderId/files/:name/*` through `resolveServableAttachmentPath` plus the same extension/regular-file gate on the symlink's resolved target, so no browser request carries a path; `~/` is refused in a Docker case (the container's home is not this host's). The chat linkifier walks TEXT NODES with DOM APIs, never rebuilds sanitized markup as a string. ⚠️ An out-of-workspace path goes through the ATTACHMENT routes (`POST /api/sessions/:id/attachments` with `notify: false`), never by widening `file-content`/`file-raw` or `file-stream-manager`'s `tail -f` allowlist. ⚠️ `TEXT_ATTACHMENT_EXTENSIONS` IS `EDITABLE_EXTENSIONS` (never a second list), and widening READ must never widen RUN: `html`/`htm`/`svg` stay download-only, other text is inert `text/plain`+`nosniff`. ⚠️ The ONE place HTML renders is `/html-view//...` (`html-view-routes.ts`, `html-view-capabilities.ts`): `POST /api/sessions/:id/html-view` mints a capability for the file's own directory, every response carries `Content-Security-Policy: sandbox` WITHOUT `allow-same-origin` (opaque origin even as a top-level tab, so the page can never read Codeman or drive its API) and with NO fetch directive (open network, owner decision 2026-10-13: never restrict it or add `allow-same-origin` without the owner), and only non-dot, asset-allowlisted files under that directory are served (GET/HEAD only; capabilities revoked with the web-tab ones on logout). That tree is served recursively, so minting is refused for a hidden directory or a broad root (`isBroadHtmlViewRoot`); the preview renders by default and the **Page** pill (or a refused mint) falls through to the source view and its Edit button. Never serve HTML same-origin anywhere else. Tests: `test/routes/html-view-routes.test.ts`. Media extensions are single-sourced in `attachment-registry.ts`. ⚠️ **XLSX previews parse in the BROWSER**, never on the server; the admission limits that keep a hostile workbook from freezing the worker live in `src/web/CLAUDE.md` (Spreadsheet preview). → [architecture-invariants#file-path-links-terminal--response-viewer](docs/architecture-invariants.md#file-path-links-terminal--response-viewer) diff --git a/README.md b/README.md index f496be07..9053d767 100644 --- a/README.md +++ b/README.md @@ -274,7 +274,7 @@ Hit start — Codeman spawns the CLI via a real PTY and streams it to your brows ### 4. Talk to the agent - **Type prompts** straight into the terminal — input is delivered exactly-once even across reconnects (a dropped link never loses or double-sends a prompt). -- **Paste, drop or attach images and videos** directly into the session. They are saved in a hidden `.codeman-uploads/` folder in the workspace (it ignores itself in git and is swept after 7 days), and the file's path lands on your prompt, unsent. +- **Paste, drop or attach images, videos and documents** (PDF, Office files, zip and plain text) directly into the session. They are saved in a hidden `.codeman-uploads/` folder in the workspace (it ignores itself in git and is swept after 7 days), and the file's path lands on your prompt, unsent. - **Voice input** — `Ctrl+Shift+V` (Deepgram Nova-3, or this machine's Claude Code login with no API key; auto-silence stop). - **Attachments** — register external files/docs and preview Office/PDF inline; any file path an agent prints is clickable, in the terminal and in the chat view, including `~/` and relative paths and paths broken across lines. An `.html` report opens as a real page, Markdown opens rendered, and a folder opens as a grid of its files. - **When it needs you** — the tab turns yellow (waiting for input) or red (a question is blocking). The **Approvals Inbox** _(opt-in)_ queues every pending prompt across sessions, answerable from the header bell or the phone home screen, and 🧠 **Read My Mind** _(opt-in)_ drafts your next prompt from the case's goals and recent work. diff --git a/README.zh-CN.md b/README.zh-CN.md index 38dd8e25..5d69ffba 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -276,7 +276,7 @@ codeman service install # systemd/launchd 服务：重启后自动回 ### 4. 与智能体对话 - **直接在终端输入提示** —— 即使跨越重连，输入也是精确一次送达（连接中断绝不会丢失或重复发送提示）。 -- **粘贴、拖放或附加图片和视频**，直接进入会话。它们保存在工作区里一个隐藏的 `.codeman-uploads/` 文件夹中（它在 git 里会忽略自己，7 天后被清理），文件路径会落在你的提示里，但不会自动发送。 +- **粘贴、拖放或附加图片、视频和文档**（PDF、Office 文件、zip 和纯文本），直接进入会话。它们保存在工作区里一个隐藏的 `.codeman-uploads/` 文件夹中（它在 git 里会忽略自己，7 天后被清理），文件路径会落在你的提示里，但不会自动发送。 - **语音输入** —— `Ctrl+Shift+V`（Deepgram Nova-3，或者直接用这台机器的 Claude Code 登录、不需要任何 API key；自动静音停止）。 - **附件** —— 注册外部文件/文档，并内联预览 Office/PDF；智能体打印出的任何文件路径都可以点击，终端里和对话视图里都行，包括 `~/` 路径、相对路径和被折成多行的路径。`.html` 报告会以真实网页打开，Markdown 会渲染后打开，文件夹则以其文件的网格打开。 - **需要你的时候** —— 标签会变黄（等待输入）或变红（有个问题挡住了它）。**审批收件箱（Approvals Inbox）**（可选启用）把所有会话里等着你的提示排成一个队列，可以从页头的铃铛或手机首页直接作答；🧠 **Read My Mind**（可选启用）会根据这个 case 的目标和最近的工作替你起草下一条提示。 diff --git a/docs/api-reference.md b/docs/api-reference.md index db7ac4d2..3353cf7f 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -600,6 +600,21 @@ beside the 30 per minute the image path keeps; without the flag the route is byte-identical to the image-only version, same 50 MB image cap and same 30/min bucket. The response shape is unchanged (`path`, `filename`). +Beyond images, the route also accepts documents a user hands an agent to read, +admitted by the name's extension (the Content-Type a phone attaches to a `.md` +or `.csv` is unreliable): the office formats `.pdf`, `.docx`, `.pptx`, `.xlsx`, +`.zip` and the text family `.txt`, `.md`, `.csv`, `.tsv`, `.json`, `.log`, +`.yaml`, `.yml`, `.xml`. A document is buffered under the same 50 MB image cap, +its bytes are checked against the extension (a magic-number signature, or UTF-8 +with no NUL byte for text) and answered 415 on a mismatch, and it is saved under +a sanitized stem of its name, so `Q3 report.pdf` is findable as +`paste---Q3-report.pdf`. A named type that is neither an image nor a +document (a script, an HTML page, a `.sh`) is refused 400 `Unsupported file +type`. That refusal is a scope choice, not a security property: `file-raw` +already serves non-media text inert (`text/plain`, `nosniff`), so this just +keeps the server from storing what it would serve or an agent could run, and +html and svg are the ones deliberately kept out. + ## Rendered HTML files An `.html`/`.htm` file renders as a page in a sandbox, in two steps diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 1836d1bf..ca7d3d68 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -1096,7 +1096,7 @@ Tests: `test/mobile-prompt-composer.test.ts` (in the CI gate, deliberately not u ### Buffers, uploads, and terminal history -Target: 20 sessions, 50 agent windows at 60fps. Limits in `src/config/`: terminal 32MB (see below), text 1MB, messages 1000, max agents 500, max sessions 50, max SSE clients 100. **Terminal history** (`src/config/terminal-history.ts`, COD-80): tmux history-limit 100k lines, PTY buffer 32MB max / 24MB trim (env `CODEMAN_MAX_TERMINAL_BUFFER`/`CODEMAN_TRIM_TERMINAL_TO`; the env-derived trim is clamped ≤75% of max — trim ≥ max would disable `BufferAccumulator` trimming entirely = unbounded memory); browser xterm scrollback stays a separate hardcoded 50k (`DEFAULT_SCROLLBACK` in constants.js — 100k/tab is a mobile-memory hazard). tmux <3.7 allocates history at pane creation, so `createSession()` sets the global default in the same command queue immediately before `new-session`; tmux 3.7+ instead creates the session and targets only that pane, because changing the global option can resize and trim unrelated live panes. A settings change resizes tracked panes only on 3.7+ and otherwise affects future panes; no version can recover lines already evicted. Settings keys `terminalScrollbackLines`/`terminalBufferMaxBytes`/`terminalBufferTrimBytes` are schema-validated but inert (only `tmuxHistoryLimit` is wired); `buffer-limits.ts` re-exports the defaults. Text/message limits are env-overridable too (`CODEMAN_MAX_TEXT_OUTPUT`/`CODEMAN_TRIM_TEXT_TO`/`CODEMAN_MAX_MESSAGES`). **Image upload** (`image-input.js` / `config/buffer-limits.ts`): up to `_maxBatchImages` 20 images/batch (bounded concurrency 3), per-file `MAX_PASTE_IMAGE_BYTES` 50MB (env `CODEMAN_MAX_PASTE_IMAGE_BYTES`); the mobile camera-roll picker auto-downscales to fit before upload. A camera-roll VIDEO (`?kind=video` on the same route, set by the client from the file's MIME type) is never buffered: it streams to disk under `MAX_PASTE_VIDEO_BYTES` 1GB (env `CODEMAN_MAX_PASTE_VIDEO_BYTES`), sniffed from the stream's first bytes (ISO BMFF / QuickTime atom, EBML for webm, Ogg for ogv) and unlinked on any failure. The client keys the flag on the MIME type, falling back to the name's extension because some Android file managers hand over a File with none; that classifier (`_promptAttachKind()`, image-input.js) is the ONE filter every pick and paste surface reads: Attach, drop, the Compose and paste-dialog pickers and the clipboard handlers. ⚠️ The flag is the ONLY way onto that path and admits video types alone (the registry's `VIDEO_ATTACHMENT_EXTENSIONS`, single-sourced so an uploaded video also previews: mp4/webm/mov/m4v/ogv; 3gp/mkv are out), so an image can never ride the larger cap through memory; without it the route is byte-identical to the image-only version. **HEIC paste uploads** (#151): converted server-side to JPEG in a `worker_threads` worker (`web/heic-jpeg-worker.ts`, resourceLimits + 30s timeout) gated by `runWithConversionLimit()`; detection is magic-byte based (covers Android/MIUI HEIFs mislabeled as JPEG); headers declaring > 64MP are rejected 415 BEFORE decode (decompression-bomb guard). Deps: `heic-decode` + `jpeg-js`. Use `LRUMap` for bounded caches, `StaleExpirationMap` for TTL cleanup. Anti-flicker pipeline: `docs/terminal-anti-flicker.md`. +Target: 20 sessions, 50 agent windows at 60fps. Limits in `src/config/`: terminal 32MB (see below), text 1MB, messages 1000, max agents 500, max sessions 50, max SSE clients 100. **Terminal history** (`src/config/terminal-history.ts`, COD-80): tmux history-limit 100k lines, PTY buffer 32MB max / 24MB trim (env `CODEMAN_MAX_TERMINAL_BUFFER`/`CODEMAN_TRIM_TERMINAL_TO`; the env-derived trim is clamped ≤75% of max — trim ≥ max would disable `BufferAccumulator` trimming entirely = unbounded memory); browser xterm scrollback stays a separate hardcoded 50k (`DEFAULT_SCROLLBACK` in constants.js — 100k/tab is a mobile-memory hazard). tmux <3.7 allocates history at pane creation, so `createSession()` sets the global default in the same command queue immediately before `new-session`; tmux 3.7+ instead creates the session and targets only that pane, because changing the global option can resize and trim unrelated live panes. A settings change resizes tracked panes only on 3.7+ and otherwise affects future panes; no version can recover lines already evicted. Settings keys `terminalScrollbackLines`/`terminalBufferMaxBytes`/`terminalBufferTrimBytes` are schema-validated but inert (only `tmuxHistoryLimit` is wired); `buffer-limits.ts` re-exports the defaults. Text/message limits are env-overridable too (`CODEMAN_MAX_TEXT_OUTPUT`/`CODEMAN_TRIM_TEXT_TO`/`CODEMAN_MAX_MESSAGES`). **Image upload** (`image-input.js` / `config/buffer-limits.ts`): up to `_maxBatchImages` 20 images/batch (bounded concurrency 3), per-file `MAX_PASTE_IMAGE_BYTES` 50MB (env `CODEMAN_MAX_PASTE_IMAGE_BYTES`); the mobile camera-roll picker auto-downscales to fit before upload. A camera-roll VIDEO (`?kind=video` on the same route, set by the client from the file's MIME type) is never buffered: it streams to disk under `MAX_PASTE_VIDEO_BYTES` 1GB (env `CODEMAN_MAX_PASTE_VIDEO_BYTES`), sniffed from the stream's first bytes (ISO BMFF / QuickTime atom, EBML for webm, Ogg for ogv) and unlinked on any failure. The client keys the flag on the MIME type, falling back to the name's extension because some Android file managers hand over a File with none; that classifier (`_promptAttachKind()`, image-input.js) is the ONE filter every pick and paste surface reads: Attach, drop, the Compose and paste-dialog pickers and the clipboard handlers. ⚠️ The flag is the ONLY way onto that path and admits video types alone (the registry's `VIDEO_ATTACHMENT_EXTENSIONS`, single-sourced so an uploaded video also previews: mp4/webm/mov/m4v/ogv; 3gp/mkv are out), so an image can never ride the larger cap through memory; without it the route is byte-identical to the image-only version. A DOCUMENT (the office formats pdf/docx/pptx/xlsx/zip plus the text family txt/md/csv/tsv/json/log/yaml/yml/xml, the curated `ALLOWED_DOCUMENT_EXTS`, never `SUPPORTED_ATTACHMENT_EXTENSIONS` or `EDITABLE_EXTENSIONS`) is admitted by its name's extension and buffered like an image, judged on its bytes by the same fail-closed `imageMagicMatchesExt` (a magic-number signature, or UTF-8 without a NUL byte for the text family) and saved under a sanitized `[A-Za-z0-9._-]` stem of its name; sh/py/js/html are a deliberate SCOPE exclusion by extension (the server declines to store what `file-raw` would serve or an agent could run), not a content filter, since the agent can already write any of them. `_promptAttachKind()` returns `image`, `video` or `document`, and the server's dispatch mirrors it: an image MIME wins whatever the name says, a document extension takes the document branch, and a foreign extension with no image MIME is refused `Unsupported file type` only after the image sniff fails, so nothing the image path accepted by its bytes is lost. **HEIC paste uploads** (#151): converted server-side to JPEG in a `worker_threads` worker (`web/heic-jpeg-worker.ts`, resourceLimits + 30s timeout) gated by `runWithConversionLimit()`; detection is magic-byte based (covers Android/MIUI HEIFs mislabeled as JPEG); headers declaring > 64MP are rejected 415 BEFORE decode (decompression-bomb guard). Deps: `heic-decode` + `jpeg-js`. Use `LRUMap` for bounded caches, `StaleExpirationMap` for TTL cleanup. Anti-flicker pipeline: `docs/terminal-anti-flicker.md`. **Prompt uploads live in `/.codeman-uploads/`** (`POST /api/sessions/:id/paste-image`; the names live ONLY in `src/web/paste-image-gc.ts`): IN the workspace because that is the only path that resolves identically for a local agent and a container (only the workspace is bind-mounted, at the same absolute path; `~/.codeman` is not); hidden so it stays out of `git status` and the agent's view of the repository; FLAT because a nested `/.codeman/` IS the data dir (`join(homedir(), '.codeman')`, `src/config/instance.ts`) when the workspace is the home directory, which nothing refuses; and self-ignoring through a `.gitignore` of `*` written once with `wx`, so a file already there is the user's and stays. The pre-move `.claude-images/` receives nothing but stays readable for one release: `UPLOAD_DIR_NAMES` lists both, the hourly sweep and the delete cleanup in `cleanupSession()` go through `uploadDirs()`, and the image watcher's ignore filter reads the names. ⚠️ `uploadDirs()` is async: the working directory is a user-chosen path, so it is probed BOUNDED first (`probePathKind()`, the #516 rule; `unknown` is skipped and never touched, the hourly sweep keeps the stall cap and `cleanupSession()` passes `pastCap`, since it acts on one path at the user's request), and the lstat and realpath after it are `fs.promises` calls, never sync, because the sweep runs 30 s after boot and hourly over every live session and a linked case on a dead mount would otherwise freeze the whole server. It returns only REAL directories (lstat at the leaf, so a planted `.codeman-uploads -> /other-case/.codeman-uploads` is never listed: `readdir` follows a link to a directory, and the sweep would otherwise age out the other case's uploads through it), none that is or contains `getDataDir()` (contrived, `CODEMAN_INSTANCE=uploads` makes `~/.codeman-uploads` the data dir of a home workspace and `CODEMAN_DATA_DIR` can point inside one, but it is one check next to a recursive delete; one strictly INSIDE the data dir is listed, since it only ever holds uploads and a workspace like the `~/.codeman/app` that `install.sh` clones would otherwise never have its uploads collected), and nothing for a remote (SSH) session, whose `workingDir` is the remote path and would otherwise name a same-named LOCAL directory for the sweep and the delete. The check is made when the directories are listed: a same-user process that swaps a listed directory for a link during the sweep's awaits is accepted (Node has no `openat()`, and that actor already writes anywhere this process can). The route refuses a remote (SSH) session with 400 before any disk touch: its `workingDir` is the remote path, so the file would land on THIS host where the agent cannot read it. ⚠️ And because a live session's workspace can go onto a network mount that later stops answering, the route probes `session.workingDir` BOUNDED (`probePathKind(..., { pastCap: true })`) before the first disk touch, refusing an `unknown` mount through `describeUnknownPath()` (OPERATION_FAILED) exactly as `POST /api/sessions` does, so a mount already unreachable cannot freeze a libuv worker on every upload (`ensureUploadDir()`'s own lstat/mkdir then start on a reachable path; a mount that dies between the probe and that write is the same residual race `POST /api/sessions` accepts, since a pre-check cannot bound the write that follows it). Tests: `test/paste-image-gc.test.ts`, `test/paste-image-dir-shared.test.ts`, the paste-image block of `test/routes/session-routes.test.ts`, `test/routes/session-create-unreachable-path.test.ts`. diff --git a/docs/wiki/Mobile-Guide.md b/docs/wiki/Mobile-Guide.md index f61f5429..69c4a80f 100644 --- a/docs/wiki/Mobile-Guide.md +++ b/docs/wiki/Mobile-Guide.md @@ -66,7 +66,8 @@ autocorrect: Enter adds a new line, and only Send delivers the text, as one past by Enter, so your line breaks reach the agent intact. Anything already typed on the terminal prompt moves into the editor when it opens. Drafts are kept per session and in memory only, so switching tabs keeps them and a page reload forgets them; a dot on the key shows a draft -is parked. The editor's Image button attaches photos or videos and puts their paths into the draft. +is parked. The editor's Attach button attaches photos, videos or documents and puts their paths +into the draft. Destructive commands need a double press, so you cannot fire `/clear` with a stray thumb. On Codex sessions the bar also shows `⇧←` and `⇧→`, the Shift-modified arrows Codex binds to editing the last queued message and walking the prompt stack. @@ -103,12 +104,16 @@ into the Run dropdown. ## The Attach button The toolbar's **Attach** button (the picture icon beside the mic) is the keyboard-down way to -hand the agent a photo or a video. One tap opens the camera and photo library sheet, nothing -else: no editor, no keyboard. The chosen files upload and their paths land on the prompt, so -you then tap the terminal, write the rest of your message and press Enter. Videos (mp4, webm, -mov, m4v, ogv) upload as recorded, up to 1 GB each by default -(`CODEMAN_MAX_PASTE_VIDEO_BYTES` on the server changes it), so a long 4K clip takes a moment -on the way up. +hand the agent a photo, a video or a document. One tap opens the system's sheet (camera, +photo library, or a file from Files or a cloud drive), nothing else: no editor, no keyboard. +The chosen files upload and their paths land on the prompt, so you then tap the terminal, +write the rest of your message and press Enter. Videos (mp4, webm, mov, m4v, ogv) +upload as recorded, up to 1 GB each by default (`CODEMAN_MAX_PASTE_VIDEO_BYTES` on the +server changes it), so a long 4K clip takes a moment on the way up. Documents are PDFs, +Office files (docx, pptx, xlsx), zips and plain-text formats such as txt, md, csv and json; +a document keeps the plain-letters part of its name in the saved file, so the agent and you +can tell the uploads apart. See [Working With Files](Working-With-Files) for where they land and how long they +stay. If the keyboard was up when you tapped Attach, iOS closes it to show the photo sheet and does not bring it back, so the tap on the terminal is how you get it again. That is the system's @@ -120,7 +125,7 @@ anything you had already typed, so you can write in front of a path, fix a word, **Use terminal keyboard** hands a single-line message back the other way, onto the prompt, so neither surface holds a half of your message. A multi-line message cannot make that trip (the prompt is a single line), so it stays parked as a draft and the dot on the Compose key -shows it is waiting. The Compose editor's own Image button remains for the times you are +shows it is waiting. The Compose editor's own Attach button remains for the times you are already writing in the editor. ## Tapping, links and copying diff --git a/docs/wiki/Settings-Reference.md b/docs/wiki/Settings-Reference.md index 62686a5d..c366df27 100644 --- a/docs/wiki/Settings-Reference.md +++ b/docs/wiki/Settings-Reference.md @@ -217,7 +217,7 @@ Some things are configured before the server starts, not in the UI: | `CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK` | Acknowledges exposing the server with no password. | | `CODEMAN_BASE_URL` | Mounts Codeman under a sub-path behind a reverse proxy that forwards the prefix unchanged. See [Remote Access](Remote-Access). | | `CODEMAN_MAX_DOWNLOAD_BYTES` | Cap on raw file bodies and downloads. 2 GB by default, `0` for none. | -| `CODEMAN_MAX_PASTE_IMAGE_BYTES` | Cap on one pasted, dropped or attached image. 50 MB by default. | +| `CODEMAN_MAX_PASTE_IMAGE_BYTES` | Cap on one pasted, dropped or attached image or document. 50 MB by default. | | `CODEMAN_MAX_PASTE_VIDEO_BYTES` | Cap on one attached video (phone Attach, drop). 1 GB by default; videos stream to disk, so this bounds disk use, not memory. | | `CODEMAN_MAX_REMOTE_FILE_SSH` | Concurrent ssh reads for files in remote cases. 4 by default. | | `CODEMAN_PATH_PROBE_TIMEOUT_MS` | How long a linked case's folder may take to answer before it is shown as unreachable. 1500 ms by default; raise it for a slow but healthy mount. | diff --git a/docs/wiki/Working-With-Files.md b/docs/wiki/Working-With-Files.md index fe783501..c71f1a37 100644 --- a/docs/wiki/Working-With-Files.md +++ b/docs/wiki/Working-With-Files.md @@ -168,17 +168,39 @@ blocks sensitive trees. In multi-user mode a non-admin gets only their own user root, because per-user spaces live inside the home directory and a home-directory root would expose everyone. -## Images into a session +## Files into a session -Paste from the clipboard or drag and drop straight onto the terminal. The image is written +Paste from the clipboard or drag and drop straight onto the terminal. The file is written where the agent can read it and the reference is inserted into your prompt. On a phone, the -toolbar's Attach button opens the camera or photo library, videos included, and puts the path -on your prompt, where it behaves like text you typed; the Image button inside the Compose -editor does the same into the editor. Dropping a video onto the terminal works the same way. +toolbar's Attach button opens the camera, photo library or files, and puts the path on your +prompt, where it behaves like text you typed; the Attach button inside the Compose editor +does the same into the editor. Dropping a video or a document onto the terminal works the +same way. -HEIC images from an iPhone are converted to JPEG on the way in. -Videos (mp4, webm, mov, m4v, ogv) are stored as recorded, up to 1 GB each by default -(`CODEMAN_MAX_PASTE_VIDEO_BYTES`); they stream to disk rather than through server memory. +Images (png, jpg, gif, webp, bmp, heic) go in after a re-encode on the device, and HEIC from +an iPhone is converted to JPEG. Videos (mp4, webm, mov, m4v, ogv) are stored as recorded, +up to 1 GB each by default (`CODEMAN_MAX_PASTE_VIDEO_BYTES`); they stream to disk rather than +through server memory. Documents are the office formats (pdf, docx, pptx, xlsx, zip) and the +text family (txt, md, csv, tsv, json, log, yaml, yml, xml). Each is checked on the way in, +against its file signature for the binary formats and against its encoding for text (UTF-8, +no NUL bytes), so a script renamed to `.pdf` is refused while a `.txt` can hold any text at +all. Files with script or page extensions (sh, py, js, html) are not accepted, since a prompt +upload is something for the agent to read, not something for Codeman to store and serve. A +document keeps a trimmed copy of its original name, letters, digits, dot, dash and underscore +with anything else turned into a dash, so `Q3 report.pdf` is findable as +`paste-