From dc33850c9c0979bf6b9c87a3064da152790f2dcd Mon Sep 17 00:00:00 2001
From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Date: Sat, 25 Apr 2026 15:17:53 -0700
Subject: [PATCH] fix(shortcuts): jump to the only terminal from non-terminal
 focus

Address codex review (round 2). The 'no-op when terminalTabs.length <= 1'
guard skipped the legitimate case of 'one terminal exists, but an editor
or browser is currently focused' - the chord did nothing instead of
jumping to that single terminal, defeating the whole point of the
shortcut. Narrow the no-op to the only true case: zero terminals, or
the only terminal already focused. Add tests for both edges.
---
 src/renderer/src/hooks/ipc-tab-switch.test.ts | 29 +++++++++++++++++++
 src/renderer/src/hooks/ipc-tab-switch.ts      |  9 +++++-
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/src/renderer/src/hooks/ipc-tab-switch.test.ts b/src/renderer/src/hooks/ipc-tab-switch.test.ts
index e57dbf37389..72723a49305 100644
--- a/src/renderer/src/hooks/ipc-tab-switch.test.ts
+++ b/src/renderer/src/hooks/ipc-tab-switch.test.ts
@@ -109,4 +109,33 @@ describe('handleSwitchTerminalTab', () => {
     expect(store.setActiveTab).toHaveBeenCalledWith('term-1')
     expect(store.setActiveTabType).toHaveBeenCalledWith('terminal')
   })
+
+  it('jumps from an editor to the only terminal when one terminal exists', () => {
+    const store = makeStore('editor')
+    store.activeFileId = 'editor-1'
+    getStateMock.mockReturnValue(store)
+    getActiveTabNavOrderMock.mockReturnValue([
+      { type: 'editor', id: 'editor-1' },
+      { type: 'terminal', id: 'term-1' },
+      { type: 'browser', id: 'browser-1' }
+    ])
+
+    expect(handleSwitchTerminalTab(1)).toBe(true)
+    expect(store.setActiveTab).toHaveBeenCalledWith('term-1')
+    expect(store.setActiveTabType).toHaveBeenCalledWith('terminal')
+  })
+
+  it('returns false when the only terminal is already active', () => {
+    const store = makeStore('terminal')
+    store.activeTabId = 'term-1'
+    getStateMock.mockReturnValue(store)
+    getActiveTabNavOrderMock.mockReturnValue([
+      { type: 'terminal', id: 'term-1' },
+      { type: 'editor', id: 'editor-1' }
+    ])
+
+    expect(handleSwitchTerminalTab(1)).toBe(false)
+    expect(store.setActiveTab).not.toHaveBeenCalled()
+    expect(store.setActiveTabType).not.toHaveBeenCalled()
+  })
 })
diff --git a/src/renderer/src/hooks/ipc-tab-switch.ts b/src/renderer/src/hooks/ipc-tab-switch.ts
index 5c3954eebc1..e8f26687e3c 100644
--- a/src/renderer/src/hooks/ipc-tab-switch.ts
+++ b/src/renderer/src/hooks/ipc-tab-switch.ts
@@ -55,7 +55,7 @@ export function handleSwitchTerminalTab(direction: number): boolean {
   const terminalTabs = getActiveTabNavOrder(store, worktreeId).filter(
     (entry) => entry.type === 'terminal'
   )
-  if (terminalTabs.length <= 1) {
+  if (terminalTabs.length === 0) {
     return false
   }
   const currentId =
@@ -67,6 +67,13 @@ export function handleSwitchTerminalTab(direction: number): boolean {
   // Why: when an editor/browser tab is active, jump to the first terminal on
   // forward navigation instead of skipping to index 1.
   const idx = terminalTabs.findIndex((t) => t.id === currentId)
+  // Why: only no-op when the sole terminal is already focused. With one terminal
+  // and an editor/browser active, the chord must still jump to that terminal -
+  // that is the whole point of the shortcut. The single-terminal-already-active
+  // case is the only true no-op.
+  if (terminalTabs.length === 1 && idx === 0) {
+    return false
+  }
   const currentIndex = idx === -1 && direction > 0 ? -1 : idx === -1 ? 0 : idx
   const next = terminalTabs[(currentIndex + direction + terminalTabs.length) % terminalTabs.length]
   store.setActiveTab(next.id)