diff --git a/docs/architecture/backend.md b/docs/architecture/backend.md index 6815c5c8..9c756f48 100644 --- a/docs/architecture/backend.md +++ b/docs/architecture/backend.md @@ -281,7 +281,7 @@ type piRPCWorker struct { | `/api/settings` | GET/POST | `handleGetSettings` / `handleSaveSettings` | Server-backed user settings (SQLite) | | `/api/btw` | GET | `handleGetBtw` | Resolve the btw scratch-chat session for a parent (SQLite) | | `/api/btw/new` | POST | `handleNewBtw` | Create a new btw scratch-chat session (SQLite) | -| `/api/projects` | GET/POST | `handleApiProjects` / `handleUpdateProject` | List projects + filter state (`limit`/`offset`, optional `current` priority + `sessionLimit` bundled summaries, active session IDs per project); enable/disable/register/remove, bulk enable-all/disable-all, enable-filter/disable-filter (SQLite) | +| `/api/projects` | GET/POST | `handleApiProjects` / `handleUpdateProject` | List projects + filter state (`limit`/`offset`, optional `current` priority + `sessionLimit` bundled summaries, active session IDs per project, `filtered=1` to apply the enabled-projects allowlist with the current project always kept); enable/disable/register/remove, bulk enable-all/disable-all, enable-filter/disable-filter (SQLite) | | `/api/sounds` | GET | `handleApiSounds` | List available notification sounds | | `/sounds/` | GET | `handleSounds` | Serve a sound asset (no auth) | | `/custom-themes.css` | GET | `handleCustomThemes` | User custom theme CSS | diff --git a/docs/architecture/system-overview.md b/docs/architecture/system-overview.md index f58e412b..91a9b646 100644 --- a/docs/architecture/system-overview.md +++ b/docs/architecture/system-overview.md @@ -167,6 +167,11 @@ across devices. See `internal/server/projects.go`. (no client flash) and is a no-op while the master switch is off. Manage via the index menu → **Manage Projects** (search, select/deselect-all, register, and the filter switch), backed by `GET/POST /api/projects`. +- The session sidebar (Projects tab and the Sessions tab's project switcher) + requests `GET /api/projects?filtered=1`, which applies the same allowlist + server-side — except the current session's project, which is always included so + the project you are in never disappears. The Manage Projects modal omits the + param and keeps seeing every project. ## Startup Order diff --git a/internal/server/projects.go b/internal/server/projects.go index 4102f019..3699d620 100644 --- a/internal/server/projects.go +++ b/internal/server/projects.go @@ -251,6 +251,19 @@ func (s *Server) handleApiProjects(w http.ResponseWriter, r *http.Request) { RunningSessionIDs: runningByProject[p], }) } + // filtered=1 applies the Manage Projects allowlist (used by the session + // sidebar). The current project is always kept so the project you are in + // never disappears; the modal omits the param to keep seeing everything. + if q.Get("filtered") == "1" && s.projectFilterEnabled() { + kept := make([]projectEntry, 0, len(entries)) + for _, entry := range entries { + if entry.Enabled || entry.Path == currentProject { + kept = append(kept, entry) + } + } + entries = kept + } + sort.Slice(entries, func(i, j int) bool { if (entries[i].Path == currentProject) != (entries[j].Path == currentProject) { return entries[i].Path == currentProject diff --git a/internal/server/projects_test.go b/internal/server/projects_test.go index 44ec118e..0809ba42 100644 --- a/internal/server/projects_test.go +++ b/internal/server/projects_test.go @@ -302,6 +302,65 @@ func TestHandleApiProjects(t *testing.T) { } } +func TestHandleApiProjectsFiltered(t *testing.T) { + sessionsDir := t.TempDir() + writeSessionWithCWD(t, filepath.Join(sessionsDir, "sub1"), "a.jsonl", "/home/user/project-a") + writeSessionWithCWD(t, filepath.Join(sessionsDir, "sub2"), "b.jsonl", "/home/user/project-b") + + s := &Server{db: newProjectPrefsDB(t), sessionsDir: sessionsDir, cache: sessions.NewCache(), now: time.Now} + // Seed both projects, then disable project-b. + s.syncProjectPrefs([]string{"/home/user/project-a", "/home/user/project-b"}) + if _, err := s.db.Exec("UPDATE project_prefs SET enabled = 0 WHERE project_path = ?", "/home/user/project-b"); err != nil { + t.Fatal(err) + } + + getPaths := func(url string) ([]string, int) { + t.Helper() + req := httptest.NewRequest(http.MethodGet, url, nil) + w := httptest.NewRecorder() + s.handleApiProjects(w, req) + if w.Code != http.StatusOK { + t.Fatalf("status = %d", w.Code) + } + var payload struct { + Projects []projectEntry `json:"projects"` + Total int `json:"total"` + } + if err := json.Unmarshal(w.Body.Bytes(), &payload); err != nil { + t.Fatal(err) + } + paths := make([]string, 0, len(payload.Projects)) + for _, p := range payload.Projects { + paths = append(paths, p.Path) + } + return paths, payload.Total + } + + // Master filter off: filtered=1 is a no-op. + if paths, total := getPaths("/api/projects?filtered=1"); len(paths) != 2 || total != 2 { + t.Fatalf("filter off: got %v (total %d), want both projects", paths, total) + } + + s.setProjectFilterEnabled(true) + + // Without filtered=1 (Manage Projects modal) everything still shows. + if paths, total := getPaths("/api/projects"); len(paths) != 2 || total != 2 { + t.Fatalf("no filtered param: got %v (total %d), want both projects", paths, total) + } + + // filtered=1 drops the disabled project and total reflects it. + paths, total := getPaths("/api/projects?filtered=1") + if len(paths) != 1 || paths[0] != "/home/user/project-a" || total != 1 { + t.Fatalf("filtered: got %v (total %d), want only project-a", paths, total) + } + + // The current project is kept even when disabled. + paths, total = getPaths("/api/projects?filtered=1¤t=/home/user/project-b") + if len(paths) != 2 || total != 2 || paths[0] != "/home/user/project-b" { + t.Fatalf("filtered current: got %v (total %d), want project-b first", paths, total) + } +} + func TestHandleApiProjectsPagination(t *testing.T) { sessionsDir := t.TempDir() for i := range 25 { diff --git a/web/src/components/session/SessionSidebarProjects.svelte b/web/src/components/session/SessionSidebarProjects.svelte index d927c830..38f325d8 100644 --- a/web/src/components/session/SessionSidebarProjects.svelte +++ b/web/src/components/session/SessionSidebarProjects.svelte @@ -122,6 +122,7 @@ offset: projects.length, currentProject: cwd, currentSessionLimit: sessionPageSize, + filtered: true, }); if (destroyed) return; const knownProjectPaths = new Set(projects.map((project) => project.path)); diff --git a/web/src/components/session/SessionSidebarProjects.test.js b/web/src/components/session/SessionSidebarProjects.test.js index 392104ce..70fdd06a 100644 --- a/web/src/components/session/SessionSidebarProjects.test.js +++ b/web/src/components/session/SessionSidebarProjects.test.js @@ -58,6 +58,7 @@ describe('SessionSidebarProjects', () => { offset: 0, currentProject: '/repo/pi-web', currentSessionLimit: 5, + filtered: true, }); expect(fetchSessions).toHaveBeenCalledWith({ project: '/repo/pi-web', @@ -190,6 +191,7 @@ describe('SessionSidebarProjects', () => { offset: 20, currentProject: '/repo/current', currentSessionLimit: 5, + filtered: true, }); }); diff --git a/web/src/components/session/SessionSidebarSessions.svelte b/web/src/components/session/SessionSidebarSessions.svelte index 9022ea8b..99e472eb 100644 --- a/web/src/components/session/SessionSidebarSessions.svelte +++ b/web/src/components/session/SessionSidebarSessions.svelte @@ -131,7 +131,7 @@ await tick(); projectSearchEl?.focus(); try { - const response = await fetchProjects(); + const response = await fetchProjects({ filtered: true }); projects = Array.isArray(response.projects) ? response.projects : []; } catch (err) { projects = []; diff --git a/web/src/components/session/chat/textarea-controls.js b/web/src/components/session/chat/textarea-controls.js index b9e8aba5..309ed9a0 100644 --- a/web/src/components/session/chat/textarea-controls.js +++ b/web/src/components/session/chat/textarea-controls.js @@ -1,3 +1,5 @@ +import { matchesAction } from '../../../shared/keybindings.js'; + export function setupTextareaControls({ windowImpl = window, textarea, @@ -35,11 +37,11 @@ export function setupTextareaControls({ event.preventDefault(); form?.requestSubmit?.(); } - if (event.key === 'Tab' && event.shiftKey) { + if (matchesAction('cycle-thinking-level', event)) { event.preventDefault(); getThinkingSelector()?.cycle?.(); } - if (event.ctrlKey && (event.key.toLowerCase() === 'i' || event.key.toLowerCase() === 'l')) { + if (matchesAction('open-model-selector', event)) { event.preventDefault(); getModelSelector()?.open?.(); } diff --git a/web/src/index/sessions.js b/web/src/index/sessions.js index 6ae3c418..f41149ee 100644 --- a/web/src/index/sessions.js +++ b/web/src/index/sessions.js @@ -136,7 +136,13 @@ export function defaultFetchRecent() { export function defaultCreateSession(path) { return postJSON('/api/new-session', { path }); } -export function defaultFetchProjects({ limit, offset, currentProject, currentSessionLimit } = {}) { +export function defaultFetchProjects({ + limit, + offset, + currentProject, + currentSessionLimit, + filtered, +} = {}) { const params = new URLSearchParams(); if (Number.isFinite(limit) && limit > 0) params.set('limit', String(limit)); if (Number.isFinite(offset) && offset > 0) params.set('offset', String(offset)); @@ -144,6 +150,7 @@ export function defaultFetchProjects({ limit, offset, currentProject, currentSes if (Number.isFinite(currentSessionLimit) && currentSessionLimit > 0) { params.set('sessionLimit', String(currentSessionLimit)); } + if (filtered) params.set('filtered', '1'); const qs = params.toString(); return getJSON('/api/projects' + (qs ? '?' + qs : '')); } diff --git a/web/src/routes/SessionsPage.svelte b/web/src/routes/SessionsPage.svelte index b2dee728..26d29546 100644 --- a/web/src/routes/SessionsPage.svelte +++ b/web/src/routes/SessionsPage.svelte @@ -9,6 +9,7 @@ import { createStatusEvents } from '../shared/status-events.js'; import { openSessionPalette, refreshSessionPalette } from '../shared/command-palette-runtime.js'; import { setupKeyboardNav } from '../shared/keyboard-nav.js'; + import { matchesAction } from '../shared/keybindings.js'; import { toggleTheme, syncThemeIcons } from '../shared/theme.js'; import { configureSettingsSync, @@ -253,14 +254,14 @@ } catch {} const keydown = (e) => { - if ((e.metaKey || e.ctrlKey) && e.shiftKey && e.key.toLowerCase() === 'l') { + if (matchesAction('toggle-theme', e)) { e.preventDefault(); e.stopPropagation(); toggleTheme(window, document); syncThemeIcons(document); return; } - if ((e.metaKey || e.ctrlKey) && e.key.toLowerCase() === 'k') { + if (matchesAction('open-palette', e)) { e.preventDefault(); openPalette(); return; diff --git a/web/src/session/session-globals.js b/web/src/session/session-globals.js index 64d33df0..667b54e2 100644 --- a/web/src/session/session-globals.js +++ b/web/src/session/session-globals.js @@ -10,6 +10,7 @@ import * as doneNotifier from './chat/done-notifier.js'; import * as sidebarApi from './ui/sidebar.js'; import { openSessionPalette } from '../shared/command-palette-runtime.js'; import { setupKeyboardNav } from '../shared/keyboard-nav.js'; +import { matchesAction } from '../shared/keybindings.js'; import { openShortcuts } from './session-modals.svelte.js'; import { sessionRuntime } from './session-runtime.js'; import { toggleTheme, syncThemeIcons } from '../shared/theme.js'; @@ -42,7 +43,7 @@ export function setupSessionGlobals({ windowImpl, documentImpl }) { // ── Global keyboard shortcuts ────────────────────────────────────────────── // Cmd+K — session list palette on(target, 'keydown', (e) => { - if ((e.metaKey || e.ctrlKey) && e.key.toLowerCase() === 'k') { + if (matchesAction('open-palette', e)) { e.preventDefault(); openSessionPalette(); } @@ -50,7 +51,7 @@ export function setupSessionGlobals({ windowImpl, documentImpl }) { // Cmd+B — toggle sidebar/tree on(target, 'keydown', (e) => { - if ((e.metaKey || e.ctrlKey) && e.key.toLowerCase() === 'b') { + if (matchesAction('toggle-sidebar', e)) { e.preventDefault(); const sidebar = documentImpl.getElementById('sidebar'); if (sidebarApi.isMobileLayout({ windowImpl: target })) { @@ -67,7 +68,7 @@ export function setupSessionGlobals({ windowImpl, documentImpl }) { // Cmd+T — new session on(target, 'keydown', (e) => { - if ((e.metaKey || e.ctrlKey) && e.key.toLowerCase() === 't') { + if (matchesAction('new-session', e)) { e.preventDefault(); const newBtn = documentImpl.getElementById('new-btn'); if (newBtn) newBtn.click(); @@ -80,7 +81,7 @@ export function setupSessionGlobals({ windowImpl, documentImpl }) { target, 'keydown', (e) => { - if ((e.metaKey || e.ctrlKey) && e.shiftKey && e.key.toLowerCase() === 'l') { + if (matchesAction('toggle-theme', e)) { e.preventDefault(); e.stopPropagation(); toggleTheme(target, documentImpl); @@ -92,7 +93,7 @@ export function setupSessionGlobals({ windowImpl, documentImpl }) { // Cmd+Shift+N — toggle scratchpad (right sidebar) on(target, 'keydown', (e) => { - if ((e.metaKey || e.ctrlKey) && e.shiftKey && e.key.toLowerCase() === 'n') { + if (matchesAction('toggle-scratchpad', e)) { e.preventDefault(); sessionRuntime.rightSidebar?.toggle(); } @@ -101,7 +102,7 @@ export function setupSessionGlobals({ windowImpl, documentImpl }) { // Cmd+/ — keyboard shortcuts help modal (the Svelte // component, opened via the shared sessionModals store). on(target, 'keydown', (e) => { - if ((e.metaKey || e.ctrlKey) && e.key === '/') { + if (matchesAction('open-shortcuts-help', e)) { e.preventDefault(); openShortcuts(); } diff --git a/web/src/session/ui/search-filters.js b/web/src/session/ui/search-filters.js index 931d80bd..a59c462c 100644 --- a/web/src/session/ui/search-filters.js +++ b/web/src/session/ui/search-filters.js @@ -1,3 +1,5 @@ +import { matchesAction } from '../../shared/keybindings.js'; + export function setupSessionSearchAndFilters({ documentImpl = document, getLeafId, @@ -68,14 +70,13 @@ export function setupSessionKeyboardShortcuts({ return; } - const key = e.key.toLowerCase(); - if (key === 't') { + if (matchesAction('toggle-thinking', e)) { e.preventDefault(); toggleThinking(); - } else if (key === 'o') { + } else if (matchesAction('toggle-tools', e)) { e.preventDefault(); toggleToolsVisibility(); - } else if (key === 'p') { + } else if (matchesAction('toggle-tool-outputs', e)) { e.preventDefault(); toggleToolOutputs(); } diff --git a/web/src/shared/keybindings.js b/web/src/shared/keybindings.js new file mode 100644 index 00000000..30033ab0 --- /dev/null +++ b/web/src/shared/keybindings.js @@ -0,0 +1,133 @@ +// Central registry of remappable keyboard actions. +// +// Before this module, every shortcut was an inline `e.key === '...'` check +// scattered across the nav, session, filter, and composer handlers. Those +// handlers now ask the registry whether an event matches a named action, so a +// single source of truth defines the default binding for each one — and a later +// change can layer user overrides on top without touching the handlers again. +// +// Scope: only the global / navigation / composer shortcuts live here. Structural +// modal keys (Escape-to-close, arrow navigation, Tab focus-traps) and the +// composer's Enter-to-submit stay hardcoded in their components — they are UI +// affordances, not preferences. The multi-key `g g` sequence also stays in +// keyboard-nav.js; the registry models single chords only. + +// A combo is a `+`-joined string of optional modifiers followed by one key, +// e.g. `mod+k`, `mod+shift+l`, `shift+i`, `ctrl+i`, `j`. +// +// mod → the platform command key: metaKey OR ctrlKey (⌘ on macOS, Ctrl +// elsewhere). This mirrors the existing `e.metaKey || e.ctrlKey` guards. +// ctrl → ctrlKey specifically, independent of ⌘. Used by the composer +// model-selector, which historically checked only ctrlKey. +// shift → shiftKey. alt → altKey. +// +// The final token is the key, matched case-insensitively against event.key +// (so `shift+i` matches the 'I' that Shift+i produces). + +export const KEY_ACTIONS = [ + // General + { id: 'open-palette', category: 'general', combo: 'mod+k' }, + { id: 'toggle-sidebar', category: 'general', combo: 'mod+b' }, + { id: 'new-session', category: 'general', combo: 'mod+t' }, + { id: 'toggle-theme', category: 'general', combo: 'mod+shift+l' }, + { id: 'toggle-scratchpad', category: 'general', combo: 'mod+shift+n' }, + { id: 'open-shortcuts-help', category: 'general', combo: 'mod+/' }, + { id: 'open-settings', category: 'general', combo: 'mod+,' }, + + // Navigation. The calling handler already excludes editable targets and + // command modifiers. These match the literal produced key (`j`, `k`, and the + // shifted `G`/`I`) exactly as the original `e.key === …` checks did, so + // Caps Lock and every other path behave identically. + { id: 'scroll-down', category: 'navigation', combo: 'j', literalKey: true }, + { id: 'scroll-up', category: 'navigation', combo: 'k', literalKey: true }, + { id: 'scroll-bottom', category: 'navigation', combo: 'shift+g', literalKey: true }, + { id: 'focus-composer', category: 'navigation', combo: 'shift+i', literalKey: true }, + + // Entry toggles. Their original handler matched the key alone with no + // modifier guard, so these stay key-only (`plain`) to preserve that exactly. + { id: 'toggle-thinking', category: 'toggles', combo: 't', plain: true }, + { id: 'toggle-tools', category: 'toggles', combo: 'o', plain: true }, + { id: 'toggle-tool-outputs', category: 'toggles', combo: 'p', plain: true }, + + // Composer + { id: 'cycle-thinking-level', category: 'composer', combo: 'shift+tab' }, + // Historically opened by Ctrl+I or Ctrl+L; both remain until the settings UI + // lets users pick one. + { id: 'open-model-selector', category: 'composer', combo: 'ctrl+i', aliases: ['ctrl+l'] }, +]; + +const ACTIONS_BY_ID = new Map(KEY_ACTIONS.map((a) => [a.id, a])); + +// Default combo for an action id (throws in tests via the map miss if unknown). +export function defaultCombo(actionId) { + return ACTIONS_BY_ID.get(actionId)?.combo ?? null; +} + +// Parse a combo string into required modifiers and a normalized key. +export function parseCombo(combo) { + const tokens = String(combo).split('+'); + const key = tokens[tokens.length - 1].toLowerCase(); + const mods = new Set(tokens.slice(0, -1)); + return { + key, + mod: mods.has('mod'), + ctrl: mods.has('ctrl'), + shift: mods.has('shift'), + alt: mods.has('alt'), + }; +} + +// expectedEventKey returns the KeyboardEvent.key value a combo produces, for +// combos matched by their literal key: a shifted single letter arrives +// uppercased (`shift+g` → `G`), everything else unchanged (`j` → `j`). +export function expectedEventKey(combo) { + const { key, shift } = parseCombo(combo); + if (shift && /^[a-z]$/.test(key)) return key.toUpperCase(); + return key; +} + +// comboMatchesEvent reports whether a modifier-bearing chord matches an event. +// Modifiers not named in the combo must be absent, so `mod+k` never fires for +// `mod+shift+k`. `mod` accepts meta or ctrl; `ctrl` requires ctrl specifically. +export function comboMatchesEvent(combo, event) { + const want = parseCombo(combo); + const hasCommand = Boolean(event.metaKey || event.ctrlKey); + + if (want.mod) { + if (!hasCommand) return false; + } else if (want.ctrl) { + if (!event.ctrlKey) return false; + } else if (hasCommand || event.altKey) { + // Plain chords (no command modifier requested) must not carry ⌘/Ctrl/Alt. + return false; + } + // Shift is only enforced where it changes meaning. For letters (compared + // case-insensitively) and named keys like Tab, event.key is the same with or + // without Shift, so `mod+k` must reject Cmd+Shift+K. For punctuation, + // event.key is already the shifted result — some layouts need Shift to type + // `/` at all — so an unrequested Shift there is layout noise, not a + // different chord. + const shiftChangesMeaning = /^[a-z]$/.test(want.key) || want.key.length > 1; + if (want.shift && !event.shiftKey) return false; + if (!want.shift && event.shiftKey && shiftChangesMeaning) return false; + if (want.alt !== Boolean(event.altKey)) return false; + return String(event.key).toLowerCase() === want.key; +} + +// matchesAction reports whether an event triggers the named action under its +// current binding. Plain-key actions (`plain: true`) compare only the key, so +// the caller's own editable/modifier guard stays authoritative — preserving the +// exact behavior of the pre-registry handlers. +export function matchesAction(actionId, event, bindings = {}) { + const action = ACTIONS_BY_ID.get(actionId); + if (!action) return false; + const combo = bindings[actionId] || action.combo; + if (action.literalKey) { + return event.key === expectedEventKey(combo); + } + if (action.plain) { + return String(event.key).toLowerCase() === parseCombo(combo).key; + } + if (comboMatchesEvent(combo, event)) return true; + return (action.aliases || []).some((alias) => comboMatchesEvent(alias, event)); +} diff --git a/web/src/shared/keybindings.test.js b/web/src/shared/keybindings.test.js new file mode 100644 index 00000000..d5ff69a0 --- /dev/null +++ b/web/src/shared/keybindings.test.js @@ -0,0 +1,147 @@ +import { describe, it, expect } from 'vitest'; +import { + KEY_ACTIONS, + defaultCombo, + parseCombo, + comboMatchesEvent, + expectedEventKey, + matchesAction, +} from './keybindings.js'; + +const ev = (key, mods = {}) => ({ + key, + metaKey: false, + ctrlKey: false, + shiftKey: false, + altKey: false, + ...mods, +}); + +describe('parseCombo', () => { + it('splits modifiers and key', () => { + expect(parseCombo('mod+shift+l')).toEqual({ + key: 'l', + mod: true, + ctrl: false, + shift: true, + alt: false, + }); + }); + + it('treats a lone key as no modifiers', () => { + expect(parseCombo('j')).toMatchObject({ key: 'j', mod: false, shift: false }); + }); +}); + +describe('expectedEventKey', () => { + it('uppercases a shifted single letter', () => { + expect(expectedEventKey('shift+g')).toBe('G'); + expect(expectedEventKey('shift+i')).toBe('I'); + }); + + it('leaves bare keys unchanged', () => { + expect(expectedEventKey('j')).toBe('j'); + }); +}); + +describe('comboMatchesEvent', () => { + it('mod accepts either meta or ctrl', () => { + expect(comboMatchesEvent('mod+k', ev('k', { metaKey: true }))).toBe(true); + expect(comboMatchesEvent('mod+k', ev('k', { ctrlKey: true }))).toBe(true); + }); + + it('requires the command modifier for mod chords', () => { + expect(comboMatchesEvent('mod+k', ev('k'))).toBe(false); + }); + + it('rejects extra modifiers not named in the combo', () => { + expect(comboMatchesEvent('mod+k', ev('k', { metaKey: true, shiftKey: true }))).toBe(false); + }); + + it('distinguishes ctrl-only from mod', () => { + expect(comboMatchesEvent('ctrl+i', ev('i', { ctrlKey: true }))).toBe(true); + // ⌘I (meta, not ctrl) must not trigger a ctrl-only chord. + expect(comboMatchesEvent('ctrl+i', ev('i', { metaKey: true }))).toBe(false); + }); + + it('matches shift chords against the shifted key value', () => { + expect(comboMatchesEvent('shift+i', ev('I', { shiftKey: true }))).toBe(true); + expect(comboMatchesEvent('mod+shift+l', ev('L', { metaKey: true, shiftKey: true }))).toBe(true); + // A bare shift chord must not fire when the command modifier is held. + expect(comboMatchesEvent('shift+i', ev('I', { shiftKey: true, ctrlKey: true }))).toBe(false); + }); + + it('rejects plain chords carrying a command modifier', () => { + expect(comboMatchesEvent('shift+i', ev('I', { shiftKey: true }))).toBe(true); + expect(comboMatchesEvent('shift+i', ev('I', { shiftKey: true, metaKey: true }))).toBe(false); + }); + + it('is case-insensitive on the key', () => { + expect(comboMatchesEvent('mod+k', ev('K', { metaKey: true }))).toBe(true); + }); + + it('ignores Shift on punctuation keys (layouts where / requires Shift)', () => { + expect(comboMatchesEvent('mod+/', ev('/', { metaKey: true, shiftKey: true }))).toBe(true); + expect(comboMatchesEvent('mod+,', ev(',', { ctrlKey: true, shiftKey: true }))).toBe(true); + // Letters and named keys still enforce Shift absence. + expect(comboMatchesEvent('mod+k', ev('K', { metaKey: true, shiftKey: true }))).toBe(false); + }); +}); + +describe('matchesAction', () => { + it('matches every default binding', () => { + expect(matchesAction('open-palette', ev('k', { metaKey: true }))).toBe(true); + expect(matchesAction('toggle-theme', ev('l', { ctrlKey: true, shiftKey: true }))).toBe(true); + expect(matchesAction('open-settings', ev(',', { metaKey: true }))).toBe(true); + expect(matchesAction('cycle-thinking-level', ev('Tab', { shiftKey: true }))).toBe(true); + }); + + it('opens shortcuts help on layouts where / is a shifted key', () => { + expect(matchesAction('open-shortcuts-help', ev('/', { metaKey: true, shiftKey: true }))).toBe( + true, + ); + expect(matchesAction('open-shortcuts-help', ev('/', { metaKey: true }))).toBe(true); + }); + + it('honors alias bindings (Ctrl+L still opens the model selector)', () => { + expect(matchesAction('open-model-selector', ev('i', { ctrlKey: true }))).toBe(true); + expect(matchesAction('open-model-selector', ev('l', { ctrlKey: true }))).toBe(true); + }); + + it('matches plain-key toggles on the key alone', () => { + // Their handler owns the editable/modifier guard, so plain keys match + // regardless of modifiers — preserving the prior t/o/p behavior. + expect(matchesAction('toggle-thinking', ev('t'))).toBe(true); + expect(matchesAction('toggle-thinking', ev('T', { shiftKey: true }))).toBe(true); + }); + + it('enforces shift on nav chords', () => { + expect(matchesAction('scroll-down', ev('j'))).toBe(true); + expect(matchesAction('scroll-down', ev('k'))).toBe(false); + // focus-composer is shift+i: bare 'i' must not trigger it. + expect(matchesAction('focus-composer', ev('I', { shiftKey: true }))).toBe(true); + expect(matchesAction('focus-composer', ev('i'))).toBe(false); + expect(matchesAction('scroll-bottom', ev('G', { shiftKey: true }))).toBe(true); + }); + + it('applies an override binding when provided', () => { + const bindings = { 'open-palette': 'mod+p' }; + expect(matchesAction('open-palette', ev('p', { metaKey: true }), bindings)).toBe(true); + expect(matchesAction('open-palette', ev('k', { metaKey: true }), bindings)).toBe(false); + }); + + it('returns false for an unknown action', () => { + expect(matchesAction('nope', ev('k', { metaKey: true }))).toBe(false); + }); +}); + +describe('registry integrity', () => { + it('exposes a unique id and category for every action', () => { + const ids = KEY_ACTIONS.map((a) => a.id); + expect(new Set(ids).size).toBe(ids.length); + for (const a of KEY_ACTIONS) { + expect(a.category).toBeTruthy(); + expect(defaultCombo(a.id)).toBe(a.combo); + } + }); +}); diff --git a/web/src/shared/keyboard-nav.js b/web/src/shared/keyboard-nav.js index 63e96e90..32b1cff4 100644 --- a/web/src/shared/keyboard-nav.js +++ b/web/src/shared/keyboard-nav.js @@ -1,4 +1,5 @@ import { navigate } from './navigation.js'; +import { matchesAction } from './keybindings.js'; const SCROLL_AMOUNT = 300; const GG_TIMEOUT = 500; // ms window for double-tap 'gg' @@ -84,7 +85,7 @@ export function setupKeyboardNav({ // Cmd/Ctrl+, opens the global settings page (standard macOS preferences // shortcut). Works regardless of focus, like a native app. documentImpl.addEventListener('keydown', (e) => { - if ((e.metaKey || e.ctrlKey) && !e.shiftKey && !e.altKey && e.key === ',') { + if (matchesAction('open-settings', e)) { e.preventDefault(); navigate('/settings', { windowImpl }); } @@ -94,7 +95,7 @@ export function setupKeyboardNav({ if (e.metaKey || e.ctrlKey || e.altKey) return; if (isEditableTarget(documentImpl.activeElement)) return; - if (e.key === 'j') { + if (matchesAction('scroll-down', e)) { e.preventDefault(); const content = typeof documentImpl.getElementById === 'function' @@ -105,7 +106,7 @@ export function setupKeyboardNav({ } else { windowImpl.scrollBy({ top: SCROLL_AMOUNT, behavior: 'instant' }); } - } else if (e.key === 'k') { + } else if (matchesAction('scroll-up', e)) { e.preventDefault(); const content = typeof documentImpl.getElementById === 'function' @@ -137,7 +138,7 @@ export function setupKeyboardNav({ ggTimer = null; }, GG_TIMEOUT); } - } else if (e.key === 'G') { + } else if (matchesAction('scroll-bottom', e)) { e.preventDefault(); const content = typeof documentImpl.getElementById === 'function' @@ -151,7 +152,7 @@ export function setupKeyboardNav({ behavior: 'instant', }); } - } else if (e.key === 'I') { + } else if (matchesAction('focus-composer', e)) { e.preventDefault(); const el = documentImpl.querySelector(focusSelector); if (el) el.focus(); diff --git a/web/src/shared/keyboard-nav.test.js b/web/src/shared/keyboard-nav.test.js index 84a8bbd5..5694ad29 100644 --- a/web/src/shared/keyboard-nav.test.js +++ b/web/src/shared/keyboard-nav.test.js @@ -209,14 +209,19 @@ describe('setupKeyboardNav', () => { expect(win.history.pushState).toHaveBeenCalledWith({}, '', '/settings'); }); - it('does not navigate to /settings on Cmd+Shift+,', () => { + it('tolerates Shift on layouts where "," is a shifted key', () => { const doc = createMockDocument(); const win = createMockWindow(); setupKeyboardNav({ windowImpl: win, documentImpl: doc }); - doc._dispatch('keydown', { key: ',', metaKey: true, shiftKey: true }); + // On a US layout Cmd+Shift+, produces '<' — must not navigate. + doc._dispatch('keydown', { key: '<', metaKey: true, shiftKey: true }); expect(win.history.pushState).not.toHaveBeenCalled(); + + // On layouts where typing ',' itself requires Shift, the chord works. + doc._dispatch('keydown', { key: ',', metaKey: true, shiftKey: true }); + expect(win.history.pushState).toHaveBeenCalledWith({}, '', '/settings'); }); it('scrolls down on j', () => {