feat(shortcuts): rebindable previous/next session, duplicate-chord refusal, rebinds stay out of the PTY

.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.

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',
+    label: 'Previous Session',
-    displayBindings: ['Alt/Option+[', 'Alt/Option+]'],
+    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 = {
+    const SHORTCUT_ACTIONS = this._shortcutActions();
-      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(),
-    };
 
     // 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.
 

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': '向右移动当前标签页',

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 || {}) };

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;
         }

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

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'");
+    // Registry entries (so they can be rebound), matched by physical key code.
-    expect(appSource).toContain("e.code === 'BracketRight'");
+    expect(appSource).toMatch(/id: 'previous-session'[\s\S]*?code: 'BracketLeft'[\s\S]*?action: 'previousSession'/);
-    expect(appSource).toContain('this.prevSession()');
+    expect(appSource).toMatch(/id: 'next-session-alt'[\s\S]*?code: 'BracketRight'[\s\S]*?action: 'nextSession'/);
-    expect(appSource).toContain('this.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'");
+  });
 });

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('_shortcutActions() {'),
-      APP_SOURCE.indexOf('// Use capture to handle before terminal')
+      APP_SOURCE.indexOf('isUserBoundShortcutEvent(e) {')
     );
     expect(actionsBlock.length).toBeGreaterThan(0);
     expect(actionsBlock).not.toContain('copyTerminalSelection');