Merge pull request #78679 from NousResearch/bb/tab-keys-leave-pages

⌘1 / ⌃Tab return to the chat from a full-page view

apps/desktop/src/app/hooks/use-keybinds.ts

@@ -42,7 +42,7 @@ import {
 } from '@/store/profile'
 import { openFolderAsProject, requestNewWorktree } from '@/store/projects'
 import { toggleReview } from '@/store/review'
-import { setModelPickerOpen } from '@/store/session'
+import { $selectedStoredSessionId, setModelPickerOpen } from '@/store/session'
 import { reopenLastClosedTile } from '@/store/session-states'
 import {
   $switcherOpen,
@@ -62,11 +62,13 @@ import { useTheme } from '@/themes/context'
 import { requestComposerFocus, requestModelMenuToggle, requestVoiceToggle } from '../chat/composer/focus'
 import { openSession } from '../open-session'
 import {
+  $workspaceIsPage,
   AGENTS_ROUTE,
   ARTIFACTS_ROUTE,
   CRON_ROUTE,
   MESSAGING_ROUTE,
   navigateToWorkspacePage,
+  NEW_CHAT_ROUTE,
   PROFILES_ROUTE,
   sessionRoute,
   SETTINGS_ROUTE,
@@ -99,11 +101,29 @@ export function useKeybinds(deps: KeybindRuntimeDeps): void {
 
   const profileSwitchHandlers: HandlerMap = {}
 
+  // A tab key that lands on the WORKSPACE tab while a full page (skills /
+  // messaging / artifacts / a plugin route) covers it must also route back to
+  // the chat: the workspace pane is already the zone's active tab behind the
+  // page, so fronting it alone changes nothing on screen and the key reads
+  // dead. Mirrors `openSession`'s full-page rule — only a route change puts
+  // the chat back.
+  const leavePageForWorkspaceChat = (paneId: null | string) => {
+    if (paneId === 'workspace' && $workspaceIsPage.get()) {
+      const selected = $selectedStoredSessionId.get()
+
+      navigate(selected ? sessionRoute(selected) : NEW_CHAT_ROUTE)
+    }
+  }
+
   for (let slot = 1; slot <= PROFILE_SLOT_COUNT; slot += 1) {
     // ⌘1…⌘9 switch the FOCUSED zone's tab when it's a real tab strip; only a
     // single-pane (or unfocused) layout falls through to the profile switch.
     profileSwitchHandlers[`profile.switch.${slot}`] = () => {
-      if (!activateTreeTabSlot(slot)) {
+      const pane = activateTreeTabSlot(slot)
+
+      if (pane) {
+        leavePageForWorkspaceChat(pane)
+      } else {
         switchProfileToSlot(slot)
       }
     }
@@ -132,6 +152,19 @@ export function useKeybinds(deps: KeybindRuntimeDeps): void {
     goToSession(openOrAdvanceSwitcher(direction))
   }
 
+  // ⌃Tab cycles the focused session/main tab strip; only a non-tabbed focus
+  // falls through to the recent-session switcher. Landing on the workspace
+  // under a full page routes back to the chat (same as ⌘1).
+  const cycleTab = (direction: 1 | -1) => {
+    const pane = cycleTreeTabInFocusedZone(direction)
+
+    if (pane) {
+      leavePageForWorkspaceChat(pane)
+    } else {
+      stepSession(direction)
+    }
+  }
+
   const showFiles = () => {
     setFileBrowserOpen(true)
     setTerminalTakeover(false)
@@ -170,10 +203,8 @@ export function useKeybinds(deps: KeybindRuntimeDeps): void {
     },
     'session.newTab': () => deps.openNewSessionTab(),
     'session.newWindow': () => void openNewWindow(),
-    // ⌃Tab cycles the focused session/main tab strip; only a non-tabbed focus
-    // falls through to the recent-session switcher.
-    'session.next': () => void (cycleTreeTabInFocusedZone(1) || stepSession(1)),
-    'session.prev': () => void (cycleTreeTabInFocusedZone(-1) || stepSession(-1)),
+    'session.next': () => cycleTab(1),
+    'session.prev': () => cycleTab(-1),
     ...sessionSlotHandlers,
     'session.focusSearch': requestSessionSearchFocus,
     'session.togglePin': deps.toggleSelectedPin,

apps/desktop/src/components/pane-shell/tree/hovered-zone-tabs.test.ts

@@ -48,14 +48,14 @@ describe('hovered zone retargets the tab verbs', () => {
     tree.noteActiveTreeGroup('grp-main')
     tree.noteHoveredTreeGroup('grp-side')
 
-    expect(tree.activateTreeTabSlot(2)).toBe(true)
+    expect(tree.activateTreeTabSlot(2)).toBeTruthy()
     expect(activeOf('grp-side')).toBe('session-tile:c')
     // The focused zone is untouched — the pointer won the target, not both.
     expect(activeOf('grp-main')).toBe('workspace')
 
     // Same key, pointer moved: the other zone's slot 2.
     tree.noteHoveredTreeGroup('grp-main')
-    expect(tree.activateTreeTabSlot(2)).toBe(true)
+    expect(tree.activateTreeTabSlot(2)).toBeTruthy()
     expect(activeOf('grp-main')).toBe('session-tile:a')
   })
 
@@ -66,7 +66,7 @@ describe('hovered zone retargets the tab verbs', () => {
     tree.noteHoveredTreeGroup('grp-main')
     tree.noteHoveredTreeGroup(null)
 
-    expect(tree.activateTreeTabSlot(2)).toBe(true)
+    expect(tree.activateTreeTabSlot(2)).toBeTruthy()
     expect(activeOf('grp-side')).toBe('session-tile:c')
     expect(activeOf('grp-main')).toBe('workspace')
   })
@@ -77,7 +77,7 @@ describe('hovered zone retargets the tab verbs', () => {
     tree.noteActiveTreeGroup('grp-main')
     tree.noteHoveredTreeGroup('grp-side')
 
-    expect(tree.cycleTreeTabInFocusedZone(1)).toBe(true)
+    expect(tree.cycleTreeTabInFocusedZone(1)).toBeTruthy()
     expect(activeOf('grp-side')).toBe('session-tile:c')
     expect(activeOf('grp-main')).toBe('workspace')
 
@@ -98,9 +98,9 @@ describe('hovered zone retargets the tab verbs', () => {
     tree.noteActiveTreeGroup('grp-side')
     tree.noteHoveredTreeGroup(null)
 
-    expect(tree.activateTreeTabSlot(2)).toBe(true)
+    expect(tree.activateTreeTabSlot(2)).toBeTruthy()
     expect(activeOf('grp-side')).toBe('session-tile:c')
-    expect(tree.cycleTreeTabInFocusedZone(1)).toBe(true)
+    expect(tree.cycleTreeTabInFocusedZone(1)).toBeTruthy()
     expect(activeOf('grp-side')).toBe('session-tile:b')
   })
 
@@ -112,7 +112,7 @@ describe('hovered zone retargets the tab verbs', () => {
     tree.noteActiveTreeGroup(null)
     tree.noteHoveredTreeGroup(null)
 
-    expect(tree.activateTreeTabSlot(2)).toBe(true)
+    expect(tree.activateTreeTabSlot(2)).toBeTruthy()
     expect(activeOf('grp-main')).toBe('session-tile:a')
     expect(activeOf('grp-side')).toBe('session-tile:b')
   })
@@ -138,7 +138,7 @@ describe('hovered zone retargets the tab verbs', () => {
     tree.noteActiveTreeGroup(null)
     tree.noteHoveredTreeGroup('grp-files')
 
-    expect(tree.activateTreeTabSlot(2)).toBe(true)
+    expect(tree.activateTreeTabSlot(2)).toBeTruthy()
     expect(activeOf('grp-main')).toBe('session-tile:a')
     // ⌘W must not close the file tree from a rung that can't serve it.
     expect(activeOf('grp-files')).toBe('files')

apps/desktop/src/components/pane-shell/tree/store.ts

@@ -549,26 +549,29 @@ function shownPanesInGroup(group: { panes: readonly string[] }): string[] {
 /** ⌘1…⌘9: activate the Nth *visible* tab of the target zone — the first of
  *  hovered / focused / workspace that is a real tab strip (≥2 shown panes).
  *  Pointing at the sidebar (or nothing) therefore still switches main's tabs
- *  instead of dead-ending. Returns false so the caller falls back to its
+ *  instead of dead-ending. Returns the activated pane id — the caller needs to
+ *  know when the slot landed on the workspace tab (a full page covering it
+ *  must also route back to the chat) — or null so it falls back to its
  *  default (profile switch) when no zone qualifies. */
-export function activateTreeTabSlot(slot: number): boolean {
+export function activateTreeTabSlot(slot: number): null | string {
   const group = tabTargetGroup(candidate => shownPanesInGroup(candidate).length >= 2)
   const panes = group ? shownPanesInGroup(group) : []
 
   if (!group || slot < 1 || slot > panes.length) {
-    return false
+    return null
   }
 
   activateTreePane(group.id, panes[slot - 1])
 
-  return true
+  return panes[slot - 1]
 }
 
 /** ⌃Tab / ⌃⇧Tab: cycle the target zone's *visible* tabs (wrapping) — the first
  *  of hovered / focused / workspace that is a chat strip with ≥2 shown tabs.
- *  Returns false so the caller falls back to the recent-session switcher when
- *  no zone qualifies. */
-export function cycleTreeTabInFocusedZone(direction: 1 | -1): boolean {
+ *  Returns the activated pane id (see `activateTreeTabSlot` — landing on the
+ *  workspace under a full page must route back to the chat), or null so the
+ *  caller falls back to the recent-session switcher when no zone qualifies. */
+export function cycleTreeTabInFocusedZone(direction: 1 | -1): null | string {
   const group = tabTargetGroup(candidate => {
     const shown = shownPanesInGroup(candidate)
 
@@ -576,7 +579,7 @@ export function cycleTreeTabInFocusedZone(direction: 1 | -1): boolean {
   })
 
   if (!group) {
-    return false
+    return null
   }
 
   const panes = shownPanesInGroup(group)
@@ -595,7 +598,7 @@ export function cycleTreeTabInFocusedZone(direction: 1 | -1): boolean {
     setTreeGroupHeaderHidden(group.id, false)
   }
 
-  return true
+  return nextId
 }
 
 /** Remove a pane from the tree WITHOUT a dismissal record — for surfaces

apps/desktop/src/components/pane-shell/tree/tab-slot-shown.test.ts

@@ -48,13 +48,13 @@ describe('activateTreeTabSlot indexes shown panes only', () => {
   it('⌘1 is workspace and ⌘2 is the first SESSION tab when files is hidden', async () => {
     const { activeOf, tree } = await setup()
 
-    expect(tree.activateTreeTabSlot(1)).toBe(true)
+    expect(tree.activateTreeTabSlot(1)).toBe('workspace')
     expect(activeOf()).toBe('workspace')
 
-    expect(tree.activateTreeTabSlot(2)).toBe(true)
+    expect(tree.activateTreeTabSlot(2)).toBe('session-tile:a')
     expect(activeOf()).toBe('session-tile:a')
 
-    expect(tree.activateTreeTabSlot(3)).toBe(true)
+    expect(tree.activateTreeTabSlot(3)).toBe('session-tile:b')
     expect(activeOf()).toBe('session-tile:b')
   })
 
@@ -63,19 +63,19 @@ describe('activateTreeTabSlot indexes shown panes only', () => {
 
     // Shown: workspace + A + B → 3. Slot 4 would have been `B` on the raw array
     // (workspace, files, A, B) before this fix — now it correctly refuses.
-    expect(tree.activateTreeTabSlot(4)).toBe(false)
+    expect(tree.activateTreeTabSlot(4)).toBeNull()
   })
 
   it('⌃Tab cycles only visible chips', async () => {
     const { activeOf, tree } = await setup()
 
-    expect(tree.cycleTreeTabInFocusedZone(1)).toBe(true)
+    expect(tree.cycleTreeTabInFocusedZone(1)).toBe('session-tile:a')
     expect(activeOf()).toBe('session-tile:a')
 
-    expect(tree.cycleTreeTabInFocusedZone(1)).toBe(true)
+    expect(tree.cycleTreeTabInFocusedZone(1)).toBe('session-tile:b')
     expect(activeOf()).toBe('session-tile:b')
 
-    expect(tree.cycleTreeTabInFocusedZone(1)).toBe(true)
+    expect(tree.cycleTreeTabInFocusedZone(1)).toBe('workspace')
     expect(activeOf()).toBe('workspace')
   })
 })