Fix Ctrl+Tab switcher selection on release (#5116)

src/main/browser/browser-guest-ui.test.ts

@@ -420,6 +420,47 @@ describe('setupGuestShortcutForwarding', () => {
     guestOffMock = vi.fn()
   })
 
+  it('commits Ctrl+Tab switching from focused guest pages on generic release events', () => {
+    setupGuestShortcutForwarding({
+      browserTabId,
+      guest: makeGuest(),
+      resolveRenderer: () => makeRenderer()
+    })
+
+    const ctrlTabInput = { code: 'Tab', key: 'Tab', control: true, meta: false }
+    const releaseInputs: Partial<Electron.Input>[] = [
+      {
+        type: 'keyUp',
+        code: 'Control',
+        key: 'Control',
+        control: false,
+        meta: false
+      },
+      {
+        type: 'keyUp',
+        code: 'Tab',
+        key: 'Tab',
+        control: false,
+        meta: false
+      }
+    ]
+
+    for (const releaseInput of releaseInputs) {
+      rendererSendMock.mockClear()
+      const keyDownPreventDefault = triggerBeforeInput(ctrlTabInput)
+      const tabReleasePreventDefault = triggerBeforeInput({ ...ctrlTabInput, type: 'keyUp' })
+      const keyUpPreventDefault = triggerBeforeInput(releaseInput)
+
+      expect(keyDownPreventDefault).toHaveBeenCalledTimes(1)
+      expect(tabReleasePreventDefault).not.toHaveBeenCalled()
+      expect(keyUpPreventDefault).toHaveBeenCalledTimes(1)
+      expect(rendererSendMock).toHaveBeenNthCalledWith(1, 'ui:ctrlTabKeyDown', {
+        shiftKey: false
+      })
+      expect(rendererSendMock).toHaveBeenNthCalledWith(2, 'ui:ctrlTabKeyUp')
+    }
+  })
+
   it('forwards browser page zoom shortcuts from focused guest pages', () => {
     setupGuestShortcutForwarding({
       browserTabId,

src/main/browser/browser-guest-ui.ts

@@ -9,6 +9,7 @@ import {
   redactKagiSessionToken
 } from '../../shared/browser-url'
 import {
+  isRecentTabSwitcherCommitRelease,
   matchesRecentTabSwitcherChord,
   resolveWindowShortcutAction
 } from '../../shared/window-shortcut-policy'
@@ -51,10 +52,6 @@ export function resolveGuestMouseWheelZoomDirection(
   return deltaY < 0 ? 'in' : 'out'
 }
 
-function isControlKeyRelease(input: Electron.Input): boolean {
-  return input.type === 'keyUp' && (input.code === 'ControlLeft' || input.code === 'ControlRight')
-}
-
 export function setupGuestContextMenu(args: {
   browserTabId: string
   guest: Electron.WebContents
@@ -268,17 +265,18 @@ export function setupGuestShortcutForwarding(args: {
   let ctrlTabSwitching = false
   const handler = (event: Electron.Event, input: Electron.Input): void => {
     const keybindings = getKeybindings?.()
-    if (matchesRecentTabSwitcherChord(input, process.platform, keybindings)) {
+    if (
+      input.type === 'keyDown' &&
+      matchesRecentTabSwitcherChord(input, process.platform, keybindings)
+    ) {
       event.preventDefault()
-      if (input.type === 'keyDown') {
-        ctrlTabSwitching = true
-        const renderer = resolveRenderer(browserTabId)
-        renderer?.send('ui:ctrlTabKeyDown', { shiftKey: input.shift === true })
-      }
+      ctrlTabSwitching = true
+      const renderer = resolveRenderer(browserTabId)
+      renderer?.send('ui:ctrlTabKeyDown', { shiftKey: input.shift === true })
       return
     }
 
-    if (ctrlTabSwitching && isControlKeyRelease(input)) {
+    if (ctrlTabSwitching && isRecentTabSwitcherCommitRelease(input)) {
       event.preventDefault()
       ctrlTabSwitching = false
       const renderer = resolveRenderer(browserTabId)

src/main/window/createMainWindow.test.ts

@@ -456,7 +456,7 @@ describe('createMainWindow', () => {
     expect(webContents.send).toHaveBeenCalledWith('ui:jumpToTabIndex', 4)
   })
 
-  it('forwards Ctrl+Tab keydown and Ctrl release to the renderer switcher', () => {
+  it('lets main-window Ctrl+Tab flow to the renderer held switcher', () => {
     const windowHandlers: Record<string, (...args: any[]) => void> = {}
     const webContents = {
       on: vi.fn((event, handler) => {
@@ -491,52 +491,30 @@ describe('createMainWindow', () => {
     createMainWindow(null)
 
     const beforeInputEvent = windowHandlers['before-input-event']
-    const firstPreventDefault = vi.fn()
-    beforeInputEvent(
-      { preventDefault: firstPreventDefault } as never,
-      {
-        type: 'keyDown',
-        code: 'Tab',
-        key: 'Tab',
-        control: true,
-        meta: false,
-        alt: false,
-        shift: false
-      } as never
-    )
-    const secondPreventDefault = vi.fn()
-    beforeInputEvent(
-      { preventDefault: secondPreventDefault } as never,
-      {
-        type: 'keyDown',
-        code: 'Tab',
-        key: 'Tab',
-        control: true,
-        meta: false,
-        alt: false,
-        shift: true
-      } as never
-    )
-    const releasePreventDefault = vi.fn()
-    beforeInputEvent(
-      { preventDefault: releasePreventDefault } as never,
-      {
-        type: 'keyUp',
-        code: 'ControlLeft',
-        key: 'Control',
-        control: false,
-        meta: false,
-        alt: false,
-        shift: false
-      } as never
-    )
+    const dispatchInput = (input: Electron.Input): ReturnType<typeof vi.fn> => {
+      const preventDefault = vi.fn()
+      beforeInputEvent({ preventDefault } as never, input as never)
+      return preventDefault
+    }
+    const ctrlTabInput = {
+      code: 'Tab',
+      key: 'Tab',
+      control: true,
+      meta: false,
+      alt: false
+    }
+    const preventDefaults = [
+      { type: 'keyDown', shift: false },
+      { type: 'keyDown', shift: true },
+      { type: 'keyUp', shift: true },
+      { type: 'keyUp', code: 'ControlLeft', key: 'Control', control: false, shift: false }
+    ].map((input) => dispatchInput({ ...ctrlTabInput, ...input } as Electron.Input))
 
-    expect(firstPreventDefault).toHaveBeenCalledTimes(1)
-    expect(secondPreventDefault).toHaveBeenCalledTimes(1)
-    expect(releasePreventDefault).toHaveBeenCalledTimes(1)
-    expect(webContents.send).toHaveBeenNthCalledWith(1, 'ui:ctrlTabKeyDown', { shiftKey: false })
-    expect(webContents.send).toHaveBeenNthCalledWith(2, 'ui:ctrlTabKeyDown', { shiftKey: true })
-    expect(webContents.send).toHaveBeenNthCalledWith(3, 'ui:ctrlTabKeyUp')
+    for (const preventDefault of preventDefaults) {
+      expect(preventDefault).not.toHaveBeenCalled()
+    }
+    expect(webContents.send).not.toHaveBeenCalledWith('ui:ctrlTabKeyDown', expect.anything())
+    expect(webContents.send).not.toHaveBeenCalledWith('ui:ctrlTabKeyUp')
   })
 
   it('does not hardcode Ctrl+Tab when the recent-tab binding is disabled', () => {

src/main/window/createMainWindow.ts

@@ -43,10 +43,6 @@ function forceRepaint(window: BrowserWindow): void {
   }, 32)
 }
 
-function isControlKeyRelease(input: Electron.Input): boolean {
-  return input.type === 'keyUp' && (input.code === 'ControlLeft' || input.code === 'ControlRight')
-}
-
 function nativeZoomCommandMatchesKeybindings(
   direction: 'in' | 'out',
   platform: NodeJS.Platform,
@@ -655,7 +651,6 @@ export function createMainWindow(
     clearRendererRecoveryTimer()
   })
 
-  let ctrlTabSwitching = false
   mainWindow.webContents.on('before-input-event', (event, input) => {
     if (shortcutRecorderFocused) {
       return
@@ -679,23 +674,11 @@ export function createMainWindow(
       )
     }
     if (
+      input.type === 'keyDown' &&
       matchesRecentTabSwitcherChord(input, process.platform, keybindings, terminalShortcutContext)
     ) {
-      // Why: Ctrl+Tab is a held-key interaction. Route both press and release
-      // through IPC so renderer keyup suppression from preventDefault cannot
-      // leave the switcher overlay stranded.
-      event.preventDefault()
-      if (input.type === 'keyDown') {
-        ctrlTabSwitching = true
-        mainWindow.webContents.send('ui:ctrlTabKeyDown', { shiftKey: input.shift === true })
-      }
-      return
-    }
-
-    if (ctrlTabSwitching && isControlKeyRelease(input)) {
-      event.preventDefault()
-      ctrlTabSwitching = false
-      mainWindow.webContents.send('ui:ctrlTabKeyUp')
+      // Why: the held switcher commits on modifier keyup. If main prevents the
+      // keydown, Electron can suppress the renderer keyup and strand the overlay.
       return
     }
 

src/renderer/src/components/tab-bar/RecentTabSwitcher.test.tsx

@@ -0,0 +1,217 @@
+// @vitest-environment happy-dom
+
+import { act } from 'react'
+import { createRoot, type Root } from 'react-dom/client'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+import type { Tab } from '../../../../shared/types'
+import type { AppState } from '../../store/types'
+
+const { activateCyclableTabMock, getStateMock } = vi.hoisted(() => ({
+  activateCyclableTabMock: vi.fn(),
+  getStateMock: vi.fn()
+}))
+
+vi.mock('../../store', () => ({
+  useAppStore: {
+    getState: getStateMock
+  }
+}))
+
+vi.mock('../../hooks/ipc-tab-switch', () => ({
+  activateCyclableTab: activateCyclableTabMock
+}))
+
+vi.mock('@/i18n/i18n', () => ({
+  translate: (_key: string, fallback: string) => fallback
+}))
+
+import RecentTabSwitcher from './RecentTabSwitcher'
+
+const WORKTREE_ID = 'wt-1'
+const GROUP_ID = 'group-1'
+
+type CtrlTabKeyDownCallback = (data: { shiftKey: boolean }) => void
+type CtrlTabKeyUpCallback = () => void
+
+let ctrlTabKeyDownCallback: CtrlTabKeyDownCallback | null = null
+
+function makeTab(id: string, entityId: string, label: string): Tab {
+  return {
+    id,
+    entityId,
+    groupId: GROUP_ID,
+    worktreeId: WORKTREE_ID,
+    contentType: 'editor',
+    label,
+    customLabel: null,
+    color: null,
+    sortOrder: 0,
+    createdAt: 1
+  }
+}
+
+function makeStore(): AppState {
+  const tabs = [
+    makeTab('tab-a', 'file-a', 'A'),
+    makeTab('tab-b', 'file-b', 'B'),
+    makeTab('tab-c', 'file-c', 'C')
+  ]
+  return {
+    activeView: 'terminal',
+    activeWorktreeId: WORKTREE_ID,
+    activeBrowserTabId: null,
+    activeFileId: 'file-a',
+    activeGroupIdByWorktree: { [WORKTREE_ID]: GROUP_ID },
+    activeTabId: null,
+    activeTabType: 'editor',
+    browserTabsByWorktree: {},
+    groupsByWorktree: {
+      [WORKTREE_ID]: [
+        {
+          id: GROUP_ID,
+          worktreeId: WORKTREE_ID,
+          activeTabId: 'tab-a',
+          tabOrder: ['tab-a', 'tab-b', 'tab-c'],
+          recentTabIds: ['tab-c', 'tab-b', 'tab-a']
+        }
+      ]
+    },
+    openFiles: tabs.map((tab) => ({
+      id: tab.entityId,
+      worktreeId: WORKTREE_ID,
+      isDirty: false
+    })),
+    settings: { ctrlTabOrderMode: 'mru' },
+    tabBarOrderByWorktree: {},
+    tabsByWorktree: {},
+    unifiedTabsByWorktree: { [WORKTREE_ID]: tabs }
+  } as unknown as AppState
+}
+
+function installWindowApi(): void {
+  Object.defineProperty(window, 'api', {
+    configurable: true,
+    value: {
+      ui: {
+        onCtrlTabKeyDown: vi.fn((callback: CtrlTabKeyDownCallback) => {
+          ctrlTabKeyDownCallback = callback
+          return vi.fn()
+        }),
+        onCtrlTabKeyUp: vi.fn((_callback: CtrlTabKeyUpCallback) => vi.fn())
+      }
+    }
+  })
+}
+
+async function renderSwitcher(): Promise<{ container: HTMLDivElement; root: Root }> {
+  const container = document.createElement('div')
+  document.body.appendChild(container)
+  const root = createRoot(container)
+  await act(async () => {
+    root.render(<RecentTabSwitcher />)
+  })
+  return { container, root }
+}
+
+function appendTerminalTextarea(): {
+  input: HTMLTextAreaElement
+  keyDown: ReturnType<typeof vi.fn>
+  keyUp: ReturnType<typeof vi.fn>
+} {
+  const input = document.createElement('textarea')
+  input.className = 'xterm-helper-textarea'
+  const keyDown = vi.fn()
+  const keyUp = vi.fn()
+  input.addEventListener('keydown', keyDown)
+  input.addEventListener('keyup', keyUp)
+  document.body.appendChild(input)
+  return { input, keyDown, keyUp }
+}
+
+async function dispatchKeyboard(
+  target: HTMLElement,
+  type: 'keydown' | 'keyup',
+  init: KeyboardEventInit
+): Promise<KeyboardEvent> {
+  const event = new KeyboardEvent(type, {
+    bubbles: true,
+    cancelable: true,
+    ...init
+  })
+  await act(async () => {
+    target.dispatchEvent(event)
+  })
+  return event
+}
+
+function expectCommittedToTabB(): void {
+  expect(activateCyclableTabMock).toHaveBeenCalledTimes(1)
+  expect(activateCyclableTabMock.mock.calls[0][1]).toMatchObject({ key: 'tab-b', label: 'B' })
+  expect(document.body.querySelector('[role="listbox"]')).toBeNull()
+}
+
+describe('RecentTabSwitcher', () => {
+  beforeEach(() => {
+    ctrlTabKeyDownCallback = null
+    activateCyclableTabMock.mockReset()
+    getStateMock.mockReturnValue(makeStore())
+    installWindowApi()
+  })
+
+  afterEach(() => {
+    document.body.innerHTML = ''
+    vi.unstubAllGlobals()
+  })
+
+  it('commits the selected tab on modifier release before terminal input sees it', async () => {
+    const { root } = await renderSwitcher()
+
+    await act(async () => {
+      ctrlTabKeyDownCallback?.({ shiftKey: false })
+    })
+
+    const terminal = appendTerminalTextarea()
+    const event = await dispatchKeyboard(terminal.input, 'keyup', {
+      key: 'Control',
+      code: 'ControlLeft',
+      ctrlKey: false
+    })
+
+    expect(event.defaultPrevented).toBe(true)
+    expect(terminal.keyUp).not.toHaveBeenCalled()
+    expectCommittedToTabB()
+
+    await act(async () => {
+      root.unmount()
+    })
+  })
+
+  it('opens from DOM Ctrl+Tab and commits on DOM Ctrl release', async () => {
+    const { root } = await renderSwitcher()
+    const terminal = appendTerminalTextarea()
+
+    const keyDown = await dispatchKeyboard(terminal.input, 'keydown', {
+      key: 'Tab',
+      code: 'Tab',
+      ctrlKey: true
+    })
+
+    expect(keyDown.defaultPrevented).toBe(true)
+    expect(terminal.keyDown).not.toHaveBeenCalled()
+    expect(document.body.querySelector('[role="listbox"]')).not.toBeNull()
+
+    const keyUp = await dispatchKeyboard(terminal.input, 'keyup', {
+      key: 'Control',
+      code: 'ControlLeft',
+      ctrlKey: false
+    })
+
+    expect(keyUp.defaultPrevented).toBe(true)
+    expect(terminal.keyUp).not.toHaveBeenCalled()
+    expectCommittedToTabB()
+
+    await act(async () => {
+      root.unmount()
+    })
+  })
+})

src/renderer/src/components/tab-bar/RecentTabSwitcher.tsx

@@ -4,7 +4,10 @@ import { FileText, GitCompare, Globe2, TerminalSquare } from 'lucide-react'
 import { useAppStore } from '../../store'
 import { activateCyclableTab } from '../../hooks/ipc-tab-switch'
 import { getShortcutPlatform } from '../../hooks/useShortcutLabel'
-import { matchesRecentTabSwitcherChord } from '../../../../shared/window-shortcut-policy'
+import {
+  isRecentTabSwitcherCommitRelease,
+  matchesRecentTabSwitcherChord
+} from '../../../../shared/window-shortcut-policy'
 import {
   buildRecentTabSwitcherModel,
   getNextRecentTabSwitcherIndex,
@@ -18,6 +21,11 @@ type SwitcherState = {
   selectedIndex: number
 }
 
+function consumeKeyboardEvent(event: KeyboardEvent): void {
+  event.preventDefault()
+  event.stopPropagation()
+}
+
 function TabIcon({ item }: { item: RecentTabSwitcherItem }): React.JSX.Element {
   const className = 'size-4 shrink-0 text-muted-foreground'
   if (item.type === 'terminal') {
@@ -105,36 +113,32 @@ export default function RecentTabSwitcher(): React.JSX.Element | null {
         // Why: Electron's native before-input-event path is authoritative, but
         // CDP/test-dispatched keys can reach the renderer directly. Respect the
         // keybinding registry here too so tests do not bypass user customization.
-        event.preventDefault()
-        event.stopPropagation()
+        consumeKeyboardEvent(event)
         openOrAdvance(event.shiftKey ? -1 : 1)
         return
       }
-      if (!switcherRef.current || event.key !== 'Escape') {
+      if (!switcherRef.current) {
         return
       }
-      event.preventDefault()
-      cancel()
+      if (event.key === 'Escape') {
+        consumeKeyboardEvent(event)
+        cancel()
+      }
     }
     const onKeyUp = (event: KeyboardEvent): void => {
-      if (
-        !switcherRef.current ||
-        (event.code !== 'ControlLeft' && event.code !== 'ControlRight' && event.key !== 'Control')
-      ) {
+      if (!switcherRef.current || !isRecentTabSwitcherCommitRelease(event)) {
         return
       }
-      event.preventDefault()
-      event.stopPropagation()
+      consumeKeyboardEvent(event)
       commit()
     }
-    const onBlur = (): void => cancel()
     window.addEventListener('keydown', onKeyDown, { capture: true })
     window.addEventListener('keyup', onKeyUp, { capture: true })
-    window.addEventListener('blur', onBlur)
+    window.addEventListener('blur', cancel)
     return () => {
       window.removeEventListener('keydown', onKeyDown, { capture: true })
       window.removeEventListener('keyup', onKeyUp, { capture: true })
-      window.removeEventListener('blur', onBlur)
+      window.removeEventListener('blur', cancel)
     }
   }, [cancel, commit, openOrAdvance])
 

src/shared/window-shortcut-policy.test.ts

@@ -4,6 +4,7 @@ navigation, new-workspace tab routing). Splitting across files would
 fragment the test of a single pure function. */
 import { describe, expect, it } from 'vitest'
 import {
+  isRecentTabSwitcherCommitRelease,
   isWindowShortcutModifierChord,
   matchesRecentTabSwitcherChord,
   resolveWindowShortcutAction,
@@ -329,6 +330,49 @@ describe('resolveWindowShortcutAction', () => {
     expect(matchesRecentTabSwitcherChord(eventInput, 'linux')).toBe(true)
   })
 
+  it('recognizes Ctrl+Tab commit releases across Electron surfaces', () => {
+    expect(
+      isRecentTabSwitcherCommitRelease({
+        type: 'keyUp',
+        code: 'ControlLeft',
+        key: 'Control',
+        control: false
+      })
+    ).toBe(true)
+    expect(
+      isRecentTabSwitcherCommitRelease({
+        type: 'keyUp',
+        code: 'Control',
+        key: 'Control',
+        control: false
+      })
+    ).toBe(true)
+    expect(
+      isRecentTabSwitcherCommitRelease({
+        type: 'keyUp',
+        code: 'Tab',
+        key: 'Tab',
+        control: false
+      })
+    ).toBe(true)
+    expect(
+      isRecentTabSwitcherCommitRelease({
+        type: 'keyUp',
+        code: 'Tab',
+        key: 'Tab',
+        control: true
+      })
+    ).toBe(false)
+    expect(
+      isRecentTabSwitcherCommitRelease({
+        type: 'keyup',
+        code: 'ControlLeft',
+        key: 'Control',
+        ctrlKey: false
+      })
+    ).toBe(true)
+  })
+
   it('accepts all supported zoom key variants', () => {
     const zoomInCases: WindowShortcutInput[] = [
       { key: '=', meta: true, control: false, alt: false, shift: false },

src/shared/window-shortcut-policy.ts

@@ -92,6 +92,32 @@ export function matchesRecentTabSwitcherChord(
   )
 }
 
+function isControlKey(input: WindowShortcutInput): boolean {
+  return (
+    input.code === 'ControlLeft' ||
+    input.code === 'ControlRight' ||
+    input.code === 'Control' ||
+    input.key === 'Control'
+  )
+}
+
+function isTabKey(input: WindowShortcutInput): boolean {
+  return input.code === 'Tab' || input.key === 'Tab'
+}
+
+export function isRecentTabSwitcherCommitRelease(input: WindowShortcutInput): boolean {
+  if (input.type !== 'keyUp' && input.type !== 'keyup') {
+    return false
+  }
+  if (isControlKey(input)) {
+    return true
+  }
+  const control = input.control ?? input.ctrlKey
+  // Why: some Electron surfaces report the final Ctrl+Tab release as Tab
+  // keyup after Control is already up, so commit instead of stranding the UI.
+  return isTabKey(input) && control === false
+}
+
 function actionMatches(
   actionId: KeybindingActionId,
   input: WindowShortcutInput,