From 88f47754ad64390176c6b1adf00caca0ccf8fed2 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Sun, 12 Jul 2026 19:06:12 +0200 Subject: [PATCH] fix(review): wire Session Manager to /api/sessions/unified contract, stop Ctrl+K PTY leak, finish shortcut registry (PR #146) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Session Manager (COD-121/192): align _loadSessionManagerList() with the merged #139 endpoint — map UnifiedSessionItem fields (lastActivityAt epoch-ms → lastModified, optional sizeBytes/firstPrompt/name) to the history-record shape _buildHistoryItem renders; surface non-2xx / error-envelope responses as a visible message instead of a silent "No sessions found"; route clicks by liveness (live row → selectSession, history row → resumeHistorySession by conversation UUID) via a new onActivate option so a live session is never duplicate-resumed - Ctrl+K double-dispatch: gate the palette chord in attachCustomKeyEventHandler (return false on keydown) so xterm never writes 0x0b kill-line into the PTY while the palette opens; gate is registry-aware so a rebound/disabled palette shortcut restores normal terminal Ctrl+K - Shortcut registry (COD-157) finished per maintainer decision: document keydown now dispatches through getShortcutRegistry() + matchesShortcutEvent() (legacy SHORTCUTS table removed), honoring per-shortcut disable and rebinds incl. the palette chord; overrides persist via saveAppSettingsToStorage() (correct device key + cache coherence, was orphaned 'codeman:settings'); Shortcuts tab renders on open via switchSettingsTab hook; capture uses a persistent listener that ignores bare modifier keydowns (combos now capturable) and requires a Ctrl/Cmd/Alt chord; settings rows use delegated listeners instead of inline onclick (JS-string injection sink) and overrides can no longer clobber id/label/action; added the missing row + overlay CSS - matchesShortcutEvent: reject undeclared extra modifiers (Ctrl+Shift+K no longer hijacked from Firefox devtools) while keeping Ctrl/Cmd interchangeable; match physical code OR produced key for layout parity - Registry/dispatch gaps: added restore-terminal-size entry, documented Ctrl+Shift+R again in the help modal (test flipped to assert presence), Ctrl+?/Alt+? now really open the registry-driven shortcut overlay, and Escape closes it - Palette new-session pick routes through selectQuickStartCase() so the searchable combobox, dir display, and lastUsedCase stay in sync - Removed fork cherry-pick debris: dead _onSessionListMaybeChanged(), orphaned .session-row-menu CSS, nonexistent closeMobileHeaderUtilities calls - Tests: functional vm-harness coverage for the unified-list field mapping + error state + liveness routing, palette chord shift/disable/ rebind handling, override persistence round-trip, capture flow, tab render hook, and source guards for the PTY gate + registry dispatch Co-Authored-By: Claude Fable 5 --- src/web/public/app.js | 91 +++++++++----- src/web/public/index.html | 1 + src/web/public/panels-ui.js | 118 +++++++++++------- src/web/public/settings-ui.js | 107 ++++++++++++---- src/web/public/styles.css | 133 ++++++++++++++------ src/web/public/terminal-ui.js | 20 ++- test/command-palette-ui.test.ts | 129 ++++++++++++++++++- test/help-modal-shortcuts.test.ts | 3 +- test/keyboard-shortcuts.test.ts | 18 +++ test/shortcut-registry-overlay.test.ts | 165 ++++++++++++++++++++++++- 10 files changed, 645 insertions(+), 140 deletions(-) diff --git a/src/web/public/app.js b/src/web/public/app.js index 935d9900..96e546e0 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -288,6 +288,7 @@ const DEFAULT_SHORTCUTS = [ label: 'Show Shortcuts', bindings: [ { modifiers: ['ctrl'], key: '?', code: 'Slash' }, + { modifiers: ['ctrl', 'shift'], key: '?' }, { modifiers: ['alt'], key: '?', code: 'Slash' }, ], action: 'showShortcutOverlay', @@ -337,6 +338,13 @@ const DEFAULT_SHORTCUTS = [ bindings: [{ modifiers: ['ctrl', 'shift'], key: 'V' }], action: 'toggleVoiceInput', }, + { + id: 'restore-terminal-size', + group: 'Terminal', + label: 'Restore Terminal Size', + bindings: [{ modifiers: ['ctrl', 'shift'], key: 'R' }], + action: 'restoreTerminalSize', + }, { id: 'move-tab-left', group: 'Tabs', @@ -903,21 +911,23 @@ class CodemanApp { // ═══════════════════════════════════════════════════════════════ setupEventListeners() { - // Keyboard shortcut lookup table — data-driven to avoid 12 separate if-blocks. - // Each entry: { key, altKey? (alternative key match), ctrl? (require Ctrl/Cmd), - // shift? (require Shift), action }. - const SHORTCUTS = [ - { key: '?', altKey: '/', ctrl: true, action: () => this.showHelp() }, - { key: 'w', ctrl: true, action: () => this.killActiveSession() }, - { key: 'Tab', ctrl: true, action: () => this.nextSession() }, - { key: 'l', ctrl: true, action: () => this.clearTerminal() }, - { key: 'R', ctrl: true, shift: true, action: () => this.restoreTerminalSize() }, - { key: '=', altKey: '+', ctrl: true, action: () => this.increaseFontSize() }, - { key: '-', ctrl: true, action: () => this.decreaseFontSize() }, - { key: 'V', ctrl: true, shift: true, action: () => VoiceInput.toggle() }, - { key: '{', ctrl: true, shift: true, action: () => this.moveActiveTabLeft() }, - { key: '}', ctrl: true, shift: true, action: () => this.moveActiveTabRight() }, - ]; + // Action name → handler map for the shortcut registry (DEFAULT_SHORTCUTS + + // user overrides from settings.shortcutOverrides, merged by + // 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(), + }; // Use capture to handle before terminal document.addEventListener('keydown', (e) => { @@ -937,6 +947,7 @@ class CodemanApp { if (this.attachmentHistoryDrawerOpen) this.closeAttachmentHistory(); this.closeSessionManager(); this.closeCommandPalette?.(); + this.closeShortcutOverlay?.(); } // Option/Alt session navigation uses physical key CODES, not e.key, so macOS @@ -966,14 +977,18 @@ class CodemanApp { } } - // Match against shortcut table - for (const s of SHORTCUTS) { - const keyMatch = e.key === s.key || (s.altKey && e.key === s.altKey); - const ctrlMatch = s.ctrl ? (e.ctrlKey || e.metaKey) : true; - const shiftMatch = s.shift ? e.shiftKey : !e.shiftKey; - if (keyMatch && ctrlMatch && shiftMatch) { + // Match against the shortcut registry so user rebinds and per-shortcut + // disables (App Settings → Shortcuts) take effect. Every dispatchable + // binding requires Ctrl/Cmd/Alt (capture enforces the same), so plain + // typing exits early without touching the registry. + if (!e.ctrlKey && !e.metaKey && !e.altKey) return; + for (const shortcut of this.getShortcutRegistry()) { + if (shortcut.disabled || !shortcut.action) continue; + const action = SHORTCUT_ACTIONS[shortcut.action]; + if (!action) continue; + if (this.matchesShortcutEvent(e, shortcut)) { e.preventDefault(); - s.action(); + action(); return; } } @@ -4360,21 +4375,35 @@ class CodemanApp { return DEFAULT_SHORTCUTS.map((shortcut) => { const override = shortcutOverrides[shortcut.id]; if (!override) return shortcut; - return { ...shortcut, ...override }; + // Only binding-shaped fields may come from storage — id/label/group/action + // stay trusted so persisted data can never redirect a shortcut's action or + // spoof another row in the settings/overlay renderers. + const merged = { ...shortcut }; + if (Array.isArray(override.bindings)) { + merged.bindings = override.bindings; + delete merged.displayBindings; // show the override, not the stale default label + } + if (typeof override.disabled === 'boolean') merged.disabled = override.disabled; + return merged; }); } matchesShortcutEvent(e, shortcut) { - if (!shortcut.bindings) return false; + if (!shortcut || !Array.isArray(shortcut.bindings)) return false; return shortcut.bindings.some((binding) => { const mods = binding.modifiers || []; - if (mods.includes('ctrl') && !e.ctrlKey) return false; - if (mods.includes('meta') && !e.metaKey) return false; - if (mods.includes('shift') && !e.shiftKey) return false; - if (mods.includes('alt') && !e.altKey) return false; - if (!mods.includes('ctrl') && !mods.includes('meta') && (e.ctrlKey || e.metaKey)) return false; - if (binding.code) return e.code === binding.code; - if (binding.key) return e.key === binding.key || e.key.toLowerCase() === binding.key.toLowerCase(); + // Ctrl and Cmd are interchangeable as the primary modifier (parity with + // the legacy shortcut table), but every OTHER pressed modifier must be + // declared by the binding — a plain Ctrl+K binding must not also swallow + // Ctrl+Shift+K (the Firefox devtools chord). + 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; + // Match the physical key when the binding pins one (layout-independent), + // or the produced character otherwise (layout-dependent keys like '+'). + 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; }); } diff --git a/src/web/public/index.html b/src/web/public/index.html index a3d68fd9..b48e4c08 100644 --- a/src/web/public/index.html +++ b/src/web/public/index.html @@ -597,6 +597,7 @@
Ctrl+L
Clear Terminal
Ctrl++
Increase Font
Ctrl+-
Decrease Font
+
Ctrl+Shift+R
Restore Terminal Size
Shift+Enter
Insert Newline
Ctrl+Enter
Insert Newline
Ctrl+Shift+V
Voice Input
diff --git a/src/web/public/panels-ui.js b/src/web/public/panels-ui.js index 2a22c2af..69d74bfe 100644 --- a/src/web/public/panels-ui.js +++ b/src/web/public/panels-ui.js @@ -257,9 +257,28 @@ Object.assign(CodemanApp.prototype, { shouldOpenCommandPaletteFromShortcut(e) { if (!e) return false; - const key = (e.key || '').toLowerCase(); - if (key !== 'k' && e.code !== 'KeyK') return false; - if (!(e.metaKey || e.ctrlKey || e.altKey)) return false; + // Every palette chord requires Ctrl/Cmd/Alt (capture enforces the same for + // rebinds), so plain typing exits before any registry work — this runs on + // the document AND xterm keydown hot paths. + if (!e.ctrlKey && !e.metaKey && !e.altKey) return false; + + // Registry-aware chord check (COD-157): honors a rebound or disabled + // palette shortcut. Falls back to the default Ctrl/Cmd/Alt+K chord when the + // registry isn't available (isolated test harnesses). + const registryAvailable = + typeof this.getShortcutRegistry === 'function' && typeof this.matchesShortcutEvent === 'function'; + const palette = registryAvailable + ? this.getShortcutRegistry().find((s) => s.id === 'command-palette') + : null; + if (palette) { + if (palette.disabled || !this.matchesShortcutEvent(e, palette)) return false; + } else { + const key = (e.key || '').toLowerCase(); + if (key !== 'k' && e.code !== 'KeyK') return false; + // Don't hijack chords with extra modifiers (Ctrl+Shift+K is the Firefox + // devtools console; matchesShortcutEvent applies the same rule above). + if (e.shiftKey) return false; + } const target = e.target; if (!target) return true; @@ -279,7 +298,6 @@ Object.assign(CodemanApp.prototype, { const search = document.getElementById('commandPaletteSearch'); if (!modal || !search) return; - this.closeMobileHeaderUtilities?.(); this.commandPaletteActiveIndex = 0; search.value = ''; modal.classList.add('active'); @@ -491,7 +509,14 @@ Object.assign(CodemanApp.prototype, { option.textContent = item.caseName; caseSelect.appendChild(option); } - caseSelect.value = item.caseName; + // selectQuickStartCase keeps the searchable combobox, dir display, and + // persisted last-used case in sync with the palette's pick (COD-151); + // fall back to a bare value set when the picker mixin isn't loaded. + if (typeof this.selectQuickStartCase === 'function') { + this.selectQuickStartCase(item.caseName); + } else { + caseSelect.value = item.caseName; + } } await this.run(); } @@ -499,9 +524,9 @@ Object.assign(CodemanApp.prototype, { // ═══════════════════════════════════════════════════════════════ // Session Manager Modal (COD-121) - // Persistent, header-reachable session list (GET /api/sessions/unified) - // reachable mid-session, with a server-side search box. Reuses the - // unit-2 history item renderer so clicking resumes/switches sessions. + // Unified session list (GET /api/sessions/unified) reachable mid-session, + // with a server-side search box. Reuses the history item renderer; clicking + // a live row switches to it, a history row resumes the conversation. // ═══════════════════════════════════════════════════════════════ async openSessionManager() { @@ -557,26 +582,13 @@ Object.assign(CodemanApp.prototype, { if (modal) modal.classList.remove('active'); }, - /** - * COD-121: live-refresh the unified session list when sessions change - * (created/updated/deleted via SSE). Only touches surfaces that are currently - * showing — the open Session Manager modal and/or the visible welcome list — - * and is debounced so an event burst collapses into one re-fetch. The current - * search query is preserved. - */ - _onSessionListMaybeChanged() { - const modal = document.getElementById('sessionManagerModal'); - if (modal && modal.classList.contains('active')) { - this._debouncedCall( - 'sessionManagerRefresh', - () => this._loadSessionManagerList(this._sessionManagerQuery || ''), - 400 - ); - } - const welcome = document.getElementById('welcomeOverlay'); - if (welcome && welcome.classList.contains('visible')) { - this._debouncedCall('welcomeHistoryRefresh', () => this.loadHistorySessions(), 600); - } + /** Replace the Session Manager list body with a single status line. */ + _setSessionManagerMessage(list, message) { + list.replaceChildren(); + const line = document.createElement('p'); + line.className = 'empty-message'; + line.textContent = message; + list.appendChild(line); }, async _loadSessionManagerList(q = '') { @@ -586,31 +598,49 @@ Object.assign(CodemanApp.prototype, { try { const url = '/api/sessions/unified?limit=200' + (q ? '&q=' + encodeURIComponent(q) : ''); const res = await fetch(url); - const data = await res.json(); - const sessions = data.data?.sessions || []; + const data = await res.json().catch(() => null); + // ApiResponse envelope: { success: true, data: { sessions, total } }. + // Surface failures instead of rendering them as an empty result set. + if (!res.ok || !data || data.success === false || !data.data) { + this._setSessionManagerMessage(list, data?.error || `Failed to load sessions (HTTP ${res.status})`); + return; + } + const sessions = data.data.sessions || []; list.replaceChildren(); if (sessions.length === 0) { - const empty = document.createElement('p'); - empty.className = 'empty-message'; - empty.textContent = q ? 'No sessions match your search' : 'No sessions found'; - list.appendChild(empty); + this._setSessionManagerMessage(list, q ? 'No sessions match your search' : 'No sessions found'); return; } for (const s of sessions) { - const item = this._buildHistoryItem(s, this.cases, { showViewAll: false }); - // COD-130: scope the modal-close to the main (resume) row only, in the - // bubble phase. The ⋯ kebab button calls stopPropagation(), so clicking - // it (or its menu) no longer closes the Session Manager modal. - item.querySelector('.history-item-main')?.addEventListener('click', () => this.closeSessionManager()); + // Adapt UnifiedSessionItem (lastActivityAt epoch-ms, optional fields) to + // the history-record shape _buildHistoryItem renders (lastModified date + // string, sizeBytes, firstPrompt). + const record = { + sessionId: s.sessionId, + workingDir: s.workingDir || '', + sizeBytes: s.sizeBytes ?? 0, + lastModified: new Date(s.lastActivityAt ?? s.createdAt ?? Date.now()).toISOString(), + firstPrompt: s.firstPrompt || s.name || '', + }; + const isLive = !!this.sessions?.has?.(s.sessionId); + const item = this._buildHistoryItem(record, this.cases, { + showViewAll: false, + onActivate: () => { + this.closeSessionManager(); + if (isLive) { + void this.selectSession(s.sessionId); + } else if (record.workingDir) { + // History rows are keyed by the Claude conversation UUID; resumed + // sessions carry theirs separately as claudeSessionId. + void this.resumeHistorySession(s.claudeSessionId || s.sessionId, record.workingDir); + } + }, + }); list.appendChild(item); } } catch (err) { console.error('[_loadSessionManagerList]', err); - list.replaceChildren(); - const errLine = document.createElement('p'); - errLine.className = 'empty-message'; - errLine.textContent = 'Failed to load sessions'; - list.appendChild(errLine); + this._setSessionManagerMessage(list, 'Failed to load sessions'); } }, diff --git a/src/web/public/settings-ui.js b/src/web/public/settings-ui.js index ccaf27da..3e8fd626 100644 --- a/src/web/public/settings-ui.js +++ b/src/web/public/settings-ui.js @@ -466,6 +466,9 @@ Object.assign(CodemanApp.prototype, { modal.querySelectorAll('.modal-tab-content').forEach(content => { content.classList.toggle('hidden', content.id !== tabName); }); + // The Shortcuts tab renders lazily so the list reflects the CURRENT + // registry (defaults + overrides) every time it is opened. + if (tabName === 'settings-shortcuts') this.renderShortcutSettingsList?.(); }, closeAppSettings() { @@ -2365,24 +2368,61 @@ Object.assign(CodemanApp.prototype, { // ─── Shortcut Settings (App Settings → Shortcuts tab) ──────────────────────── // Renders the list of shortcuts with capture buttons for key rebinding, - // and persists overrides under settings.shortcutOverrides. + // and persists overrides under settings.shortcutOverrides (saved through + // saveAppSettingsToStorage so the device key + settings cache stay coherent). renderShortcutSettingsList() { const list = document.getElementById('appSettingsShortcutsList'); if (!list) return; - const registry = this.getShortcutRegistry ? this.getShortcutRegistry() : (typeof DEFAULT_SHORTCUTS !== 'undefined' ? DEFAULT_SHORTCUTS : []); - list.innerHTML = registry.map((shortcut) => { - const bindingLabel = shortcut.displayBindings - ? shortcut.displayBindings.join(' / ') - : (shortcut.bindings || []).map((b) => [...(b.modifiers || []), b.key || b.code || ''].join('+')).join(' / '); - return `
+ const registry = this.getShortcutRegistry + ? this.getShortcutRegistry() + : typeof DEFAULT_SHORTCUTS !== 'undefined' + ? DEFAULT_SHORTCUTS + : []; + const overrides = this.readShortcutOverridesFromSettings(); + list.innerHTML = registry + .map((shortcut) => { + const bindingLabel = shortcut.displayBindings + ? shortcut.displayBindings.join(' / ') + : (shortcut.bindings || []).map((b) => [...(b.modifiers || []), b.key || b.code || ''].join('+')).join(' / '); + // Only registry entries dispatched through matchesShortcutEvent() are + // configurable; fixed keys (Escape, tab arrows, …) render read-only. + const configurable = !!shortcut.action && Array.isArray(shortcut.bindings); + const overridden = !!overrides[shortcut.id]; + const controls = configurable + ? ` + + ` + : ''; + return `
- - - + ${controls}
`; - }).join(''); + }) + .join(''); + this._wireShortcutSettingsList(list); + }, + + // Delegated handlers (no inline onclick — registry ids never land inside a + // JS string context, and the listeners survive re-renders). + _wireShortcutSettingsList(list) { + if (list.dataset.shortcutListenersAdded) return; + list.dataset.shortcutListenersAdded = 'true'; + list.addEventListener('click', (e) => { + const btn = e.target?.closest?.('[data-shortcut-action]'); + if (!btn) return; + const id = btn.closest?.('[data-shortcut-id]')?.dataset?.shortcutId; + if (!id) return; + if (btn.dataset.shortcutAction === 'capture') this.startShortcutCapture(id); + else if (btn.dataset.shortcutAction === 'reset') this.resetShortcutOverride(id); + }); + list.addEventListener('change', (e) => { + const box = e.target; + if (!box?.matches?.('[data-shortcut-action="toggle"]')) return; + const id = box.closest?.('[data-shortcut-id]')?.dataset?.shortcutId; + if (id) this.toggleShortcutEnabled(id, box.checked); + }); }, readShortcutOverridesFromSettings() { @@ -2396,45 +2436,66 @@ Object.assign(CodemanApp.prototype, { input.value = 'Press keys…'; input.focus(); this._capturingShortcutId = shortcutId; - input.addEventListener('keydown', (e) => this.onShortcutCaptureKeydown(e, shortcutId), { once: true }); + // Persistent listener (NOT {once}) — the first keydown of a combo like + // Ctrl+Shift+P is the modifier itself ('Control'), which must not end the + // capture. The first non-modifier key completes it. + const onCaptureKeydown = (e) => { + e.preventDefault(); + e.stopPropagation(); + if (e.key === 'Control' || e.key === 'Shift' || e.key === 'Alt' || e.key === 'Meta') return; + input.removeEventListener('keydown', onCaptureKeydown); + this.onShortcutCaptureKeydown(e, shortcutId); + }; + input.addEventListener('keydown', onCaptureKeydown); }, onShortcutCaptureKeydown(e, shortcutId) { e.preventDefault(); e.stopPropagation(); + this._capturingShortcutId = null; if (e.key === 'Escape') { this.renderShortcutSettingsList(); return; } - const settings = this.loadAppSettingsFromStorage(); - const shortcutOverrides = settings.shortcutOverrides || {}; + // Require a real chord: the dispatcher has no focus-target guard, so a + // bare-key binding would fire while typing in any input. + if (!e.ctrlKey && !e.metaKey && !e.altKey) { + this.renderShortcutSettingsList(); + this.showToast?.('Shortcut must include Ctrl, Cmd, or Alt', 'error'); + return; + } const modifiers = []; if (e.ctrlKey) modifiers.push('ctrl'); if (e.metaKey) modifiers.push('meta'); if (e.shiftKey) modifiers.push('shift'); if (e.altKey) modifiers.push('alt'); - shortcutOverrides[shortcutId] = { bindings: [{ modifiers, key: e.key, code: e.code }] }; + const settings = this.loadAppSettingsFromStorage(); + const shortcutOverrides = { ...(settings.shortcutOverrides || {}) }; + shortcutOverrides[shortcutId] = { + ...(shortcutOverrides[shortcutId] || {}), + bindings: [{ modifiers, key: e.key, code: e.code }], + }; settings.shortcutOverrides = shortcutOverrides; - localStorage.setItem('codeman:settings', JSON.stringify(settings)); - this._capturingShortcutId = null; + this.saveAppSettingsToStorage(settings); this.renderShortcutSettingsList(); }, resetShortcutOverride(shortcutId) { const settings = this.loadAppSettingsFromStorage(); - const shortcutOverrides = settings.shortcutOverrides || {}; + const shortcutOverrides = { ...(settings.shortcutOverrides || {}) }; delete shortcutOverrides[shortcutId]; settings.shortcutOverrides = shortcutOverrides; - localStorage.setItem('codeman:settings', JSON.stringify(settings)); + this.saveAppSettingsToStorage(settings); this.renderShortcutSettingsList(); }, toggleShortcutEnabled(shortcutId, enabled) { const settings = this.loadAppSettingsFromStorage(); - const shortcutOverrides = settings.shortcutOverrides || {}; + const shortcutOverrides = { ...(settings.shortcutOverrides || {}) }; shortcutOverrides[shortcutId] = { ...(shortcutOverrides[shortcutId] || {}), disabled: !enabled }; settings.shortcutOverrides = shortcutOverrides; - localStorage.setItem('codeman:settings', JSON.stringify(settings)); + this.saveAppSettingsToStorage(settings); + this.renderShortcutSettingsList(); }, closeAllPanels() { @@ -2442,10 +2503,6 @@ Object.assign(CodemanApp.prototype, { this.closeAppSettings(); this.cancelCloseSession(); this.closeTokenStats(); - // Mobile header utility tray closes alongside the other overlays so Escape - // (and any future closeAllPanels caller) dismisses it like every other panel - // instead of leaving it pinned open with only its toggle to close it. - this.closeMobileHeaderUtilities?.(); document.getElementById('monitorPanel').classList.remove('open'); // Collapse subagents panel (don't hide it permanently) const subagentsPanel = document.getElementById('subagentsPanel'); diff --git a/src/web/public/styles.css b/src/web/public/styles.css index d547c633..ea5dbc07 100644 --- a/src/web/public/styles.css +++ b/src/web/public/styles.css @@ -5704,49 +5704,112 @@ kbd { overflow-y: auto; } -/* COD-130: per-row ⋯ kebab context menu (appended to , fixed-positioned). - z-index must clear the Session Manager modal (.modal is z-index: 1000). */ -.session-row-menu { - position: fixed; - z-index: 10010; - min-width: 180px; - display: flex; - flex-direction: column; - gap: 2px; - padding: 4px; - background: rgba(22, 22, 28, 0.97); - backdrop-filter: blur(20px); - -webkit-backdrop-filter: blur(20px); - border: 1px solid var(--border); - border-radius: 10px; - box-shadow: 0 8px 32px rgba(0, 0, 0, 0.5), 0 2px 8px rgba(0, 0, 0, 0.3); +/* Shortcut settings rows (App Settings → Shortcuts, COD-157) */ +.shortcut-setting-row { + display: grid; + grid-template-columns: minmax(0, 1fr) minmax(0, 190px) auto auto auto; + align-items: center; + gap: 0.5rem; + padding: 0.4rem 0; + border-bottom: 1px solid rgba(255, 255, 255, 0.05); } -.session-row-menu-item { - display: flex; - flex-direction: column; - align-items: flex-start; - gap: 1px; - width: 100%; - padding: 7px 12px; - background: none; - border: none; - border-radius: var(--btn-radius); +.shortcut-setting-row:last-child { + border-bottom: none; +} + +.shortcut-setting-label { color: var(--text); - cursor: pointer; font-size: 0.8rem; - text-align: left; + overflow: hidden; + text-overflow: ellipsis; white-space: nowrap; - transition: background var(--transition-smooth); } -.session-row-menu-item:hover { - background: var(--bg-hover); -} - -.session-row-menu-sublabel { - color: var(--text-muted); +.shortcut-binding-input { + min-width: 0; + padding: 0.3rem 0.5rem; + background: rgba(255, 255, 255, 0.05); + border: 1px solid rgba(255, 255, 255, 0.08); + border-radius: var(--btn-radius); + color: var(--text-dim); + font-family: monospace; font-size: 0.72rem; + cursor: default; +} + +.shortcut-binding-input:focus { + border-color: var(--accent); + color: var(--text); + outline: none; +} + +.shortcut-capture-btn, +.shortcut-reset-btn { + padding: 0.3rem 0.6rem; + background: rgba(255, 255, 255, 0.05); + border: 1px solid rgba(255, 255, 255, 0.08); + border-radius: var(--btn-radius); + color: var(--text-dim); + font-size: 0.72rem; + cursor: pointer; + transition: all var(--transition-smooth); +} + +.shortcut-capture-btn:hover, +.shortcut-reset-btn:hover:not(:disabled) { + background: rgba(255, 255, 255, 0.09); + color: var(--text); +} + +.shortcut-reset-btn:disabled { + opacity: 0.4; + cursor: default; +} + +.shortcut-enabled-checkbox { + accent-color: var(--accent); + cursor: pointer; +} + +/* Shortcut overlay modal (registry-driven; opened with Ctrl+? / Alt+?) */ +.shortcut-overlay-modal .modal-content { + max-width: 560px; +} + +.shortcut-overlay-group { + margin-bottom: 1rem; +} + +.shortcut-overlay-group:last-child { + margin-bottom: 0; +} + +.shortcut-overlay-group-label { + margin-bottom: 0.35rem; + color: var(--text-muted); + font-size: 0.68rem; + font-weight: 600; + letter-spacing: 0.04em; + text-transform: uppercase; +} + +.shortcut-overlay-row { + display: flex; + align-items: center; + justify-content: space-between; + gap: 1rem; + padding: 0.3rem 0; +} + +.shortcut-overlay-label { + color: var(--text); + font-size: 0.8rem; +} + +.shortcut-overlay-keys { + flex-shrink: 0; + color: var(--text-dim); } .away-digest-ranges { diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index 7b287297..cf5cc362 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -134,6 +134,17 @@ Object.assign(CodemanApp.prototype, { 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 + // this gate Ctrl+K would ALSO write 0x0b (readline kill-line) into the + // live session behind the palette, truncating whatever the user had + // typed. Route through the registry-aware checker so a rebound or + // disabled palette shortcut restores normal terminal Ctrl+K. + if (ev.type === 'keydown' && this.shouldOpenCommandPaletteFromShortcut?.(ev)) { + return false; + } + // Ctrl+V / Cmd+V: intercept before xterm sends ^V to PTY. // Route through our paste trap which handles both images and text. if ((ev.ctrlKey || ev.metaKey) && ev.key === 'v' && ev.type === 'keydown') { @@ -1074,6 +1085,7 @@ Object.assign(CodemanApp.prototype, { * @param {Array} cases linked cases (for #caseName label) * @param {object} [options] * @param {boolean} [options.showViewAll=true] show "View all in folder" button in detail panel + * @param {Function} [options.onActivate] main-row click handler override (default: resume the conversation) */ _buildHistoryItem(s, cases, options) { const showViewAll = options?.showViewAll !== false; @@ -1095,10 +1107,14 @@ Object.assign(CodemanApp.prototype, { item.className = 'history-item'; item.title = s.workingDir; - // Main row: clickable surface that triggers resume + // Main row: clickable surface that triggers resume (or a caller-supplied + // activation — the Session Manager switches to live sessions instead) const mainRow = document.createElement('div'); mainRow.className = 'history-item-main'; - mainRow.addEventListener('click', () => this.resumeHistorySession(s.sessionId, s.workingDir)); + mainRow.addEventListener( + 'click', + options?.onActivate || (() => this.resumeHistorySession(s.sessionId, s.workingDir)) + ); const textCol = document.createElement('div'); textCol.className = 'history-item-text'; diff --git a/test/command-palette-ui.test.ts b/test/command-palette-ui.test.ts index 4b68ba9a..de8df0a2 100644 --- a/test/command-palette-ui.test.ts +++ b/test/command-palette-ui.test.ts @@ -107,7 +107,6 @@ function loadPaletteHarness(overrides: Record = {}) { app.cases = [{ name: 'plex-previews' }, { name: 'flux-player' }, { name: 'api-tools' }]; app.selectSession = vi.fn(); app.run = vi.fn(); - app.closeMobileHeaderUtilities = vi.fn(); app.getShortId = (id: string) => id.slice(0, 8); app.getSessionName = (session: any) => session.name || session.workingDir?.split('/').pop() || app.getShortId(session.id); @@ -177,6 +176,37 @@ describe('Command-K session palette', () => { ).toBe(true); }); + it('rejects the palette chord when extra modifiers are held (Ctrl+Shift+K is the Firefox devtools console)', () => { + const { app } = loadPaletteHarness(); + + expect( + app.shouldOpenCommandPaletteFromShortcut({ key: 'K', code: 'KeyK', ctrlKey: true, shiftKey: true, target: null }) + ).toBe(false); + }); + + it('honors a disabled or rebound palette shortcut from the registry', () => { + const { app } = loadPaletteHarness(); + + // Disabled entry → never opens, even for the default chord. + app.getShortcutRegistry = () => [ + { id: 'command-palette', disabled: true, bindings: [{ modifiers: ['ctrl'], key: 'k', code: 'KeyK' }] }, + ]; + app.matchesShortcutEvent = () => true; + expect(app.shouldOpenCommandPaletteFromShortcut({ key: 'k', code: 'KeyK', ctrlKey: true, target: null })).toBe( + false + ); + + // Rebound entry → the new chord opens, the old default no longer does. + app.getShortcutRegistry = () => [{ id: 'command-palette', bindings: [{ modifiers: ['ctrl'], code: 'KeyP' }] }]; + app.matchesShortcutEvent = (e: any, s: any) => e.code === s.bindings[0].code; + expect(app.shouldOpenCommandPaletteFromShortcut({ key: 'k', code: 'KeyK', ctrlKey: true, target: null })).toBe( + false + ); + expect(app.shouldOpenCommandPaletteFromShortcut({ key: 'p', code: 'KeyP', ctrlKey: true, target: null })).toBe( + true + ); + }); + it('opens and focuses the palette search box', () => { const { app, elements } = loadPaletteHarness(); @@ -258,6 +288,21 @@ describe('Command-K session palette', () => { expect(app.run).toHaveBeenCalledTimes(1); }); + it('routes the new-session case pick through selectQuickStartCase when the picker mixin is loaded', async () => { + const { app } = loadPaletteHarness(); + app.selectQuickStartCase = vi.fn(); + + const newSession = app.buildCommandPaletteItems('flux').find((item: any) => item.type === 'new-session'); + app.commandPaletteItems = [newSession]; + app.commandPaletteActiveIndex = 0; + await app.activateCommandPaletteItem(); + + // Keeps the searchable combobox, dir display, and lastUsedCase in sync + // instead of silently mutating the hidden native