From c7b021ca487277c322281aef040ae743681132ba Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Thu, 30 Jul 2026 21:43:15 -0500 Subject: [PATCH] fix(desktop): closing the last main tab lands on New session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The workspace pane can't leave the tree, so "close the main tab" only ever had one answer wired: shift the next stacked session in. With main as the only tab there was nothing to shift and ⌘W dead-ended on the tab the user was looking at. closeWorkspaceTab is now the one answer for every entry point — stacked session still wins, and with nothing stacked main drops to a fresh New session draft. A blank draft and a full-page view stay no-ops: a blank draft already IS the post-close state. --- apps/desktop/src/app/chat/close-tab.test.ts | 112 +++++++++++++++++--- apps/desktop/src/app/chat/close-tab.ts | 77 +++++++++----- 2 files changed, 150 insertions(+), 39 deletions(-) diff --git a/apps/desktop/src/app/chat/close-tab.test.ts b/apps/desktop/src/app/chat/close-tab.test.ts index 44368c80403..0326efebead 100644 --- a/apps/desktop/src/app/chat/close-tab.test.ts +++ b/apps/desktop/src/app/chat/close-tab.test.ts @@ -1,9 +1,30 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +const closeFocusedSessionTab = vi.fn(() => false) +const nextSessionTileForWorkspace = vi.fn<() => null | string>(() => null) +const closeSessionTile = vi.fn() +const requestFreshSession = vi.fn() + +vi.mock('@/components/pane-shell/tree/store', () => ({ + closeFocusedSessionTab: () => closeFocusedSessionTab() +})) + +vi.mock('@/store/session-states', () => ({ + closeSessionTile: (...args: unknown[]) => closeSessionTile(...args), + nextSessionTileForWorkspace: () => nextSessionTileForWorkspace() +})) + +vi.mock('@/store/profile', () => ({ + requestFreshSession: () => requestFreshSession() +})) + import { $rightRailActiveTabId } from '@/store/layout' import { $previewTabs, closeRightRail, openPreview, type PreviewTarget } from '@/store/preview' +import { $activeSessionId, $selectedStoredSessionId } from '@/store/session' -import { closeActiveTab } from './close-tab' +import { $workspaceIsPage } from '../routes' + +import { closeActiveTab, closeWorkspaceTab } from './close-tab' function fileTarget(path: string): PreviewTarget { return { @@ -16,19 +37,31 @@ function fileTarget(path: string): PreviewTarget { } } +/** Main is holding a loaded chat and nothing else is stacked with it. */ +function loadedMainOnly() { + $selectedStoredSessionId.set('stored-a') + $activeSessionId.set('runtime-a') +} + +beforeEach(() => { + vi.stubGlobal('document', { activeElement: null }) + closeRightRail() + window.localStorage.clear() + $selectedStoredSessionId.set(null) + $activeSessionId.set(null) + $workspaceIsPage.set(false) + closeFocusedSessionTab.mockReturnValue(false) + nextSessionTileForWorkspace.mockReturnValue(null) + vi.clearAllMocks() +}) + +afterEach(() => { + vi.unstubAllGlobals() + closeRightRail() + window.localStorage.clear() +}) + describe('closeActiveTab', () => { - beforeEach(() => { - vi.stubGlobal('document', { activeElement: null }) - closeRightRail() - window.localStorage.clear() - }) - - afterEach(() => { - vi.unstubAllGlobals() - closeRightRail() - window.localStorage.clear() - }) - it('closes the active file preview tab (⌘W happy path)', () => { openPreview(fileTarget('/work/notes.md'), 'manual') @@ -50,3 +83,56 @@ describe('closeActiveTab', () => { expect($previewTabs.get()).toHaveLength(0) }) }) + +/** + * The main tab's own close. The workspace pane can never leave the tree, so + * every answer here is about what FILLS it — a stacked session, or an empty + * draft. The gesture used to dead-end whenever main was the only tab. + */ +describe('closeWorkspaceTab', () => { + it('shifts the next stacked session into main', () => { + loadedMainOnly() + nextSessionTileForWorkspace.mockReturnValue('stored-b') + const load = vi.fn() + + expect(closeWorkspaceTab(load)).toBe(true) + expect(closeSessionTile).toHaveBeenCalledWith('stored-b') + expect(load).toHaveBeenCalledWith('stored-b') + // Promotion refills main — it must not ALSO blank it. + expect(requestFreshSession).not.toHaveBeenCalled() + }) + + it('drops a lone loaded main to a fresh draft', () => { + loadedMainOnly() + + expect(closeWorkspaceTab(vi.fn())).toBe(true) + expect(requestFreshSession).toHaveBeenCalledTimes(1) + }) + + it('empties main even with no session loader wired', () => { + loadedMainOnly() + + expect(closeWorkspaceTab()).toBe(true) + expect(requestFreshSession).toHaveBeenCalledTimes(1) + }) + + it('is a no-op on a blank draft — that IS the post-close state', () => { + expect(closeWorkspaceTab(vi.fn())).toBe(false) + expect(requestFreshSession).not.toHaveBeenCalled() + }) + + it('is a no-op over a full-page view, which owns no chat tab', () => { + loadedMainOnly() + $workspaceIsPage.set(true) + + expect(closeWorkspaceTab(vi.fn())).toBe(false) + expect(requestFreshSession).not.toHaveBeenCalled() + }) + + it('⌘W reaches it once the terminal, rail and zone tabs pass', () => { + loadedMainOnly() + + expect(closeActiveTab(vi.fn())).toBe(true) + expect(requestFreshSession).toHaveBeenCalledTimes(1) + }) +}) diff --git a/apps/desktop/src/app/chat/close-tab.ts b/apps/desktop/src/app/chat/close-tab.ts index 89c85259312..0d75455bd80 100644 --- a/apps/desktop/src/app/chat/close-tab.ts +++ b/apps/desktop/src/app/chat/close-tab.ts @@ -1,27 +1,68 @@ +import { mainChatOccupied } from '@/app/open-session' import { closeActiveTerminal } from '@/app/right-sidebar/terminal/terminals' +import { $workspaceIsPage } from '@/app/routes' import { closeFocusedSessionTab } from '@/components/pane-shell/tree/store' import { isFocusWithin } from '@/lib/keybinds/combo' import { $previewTabs, closeActiveRightRailTab } from '@/store/preview' +import { requestFreshSession } from '@/store/profile' +import { $activeSessionId, $selectedStoredSessionId } from '@/store/session' import { closeSessionTile, nextSessionTileForWorkspace } from '@/store/session-states' +/** + * Close the MAIN tab. The workspace pane itself can't leave the tree, so + * "closing" it means emptying it, and what fills the hole depends on what's + * stacked beside it: + * + * - session tabs stacked with it → the next one shifts INTO main (drop its + * tile, load it as the primary — the session stays alive, no busy prompt), + * - nothing stacked → main drops to a fresh "New session" draft. + * + * The second half is what makes the gesture honest when main is the ONLY tab: + * ⌘W / ⌘-click / middle-click used to be a dead key there, since the only + * available answer was "remove the pane", which this app never does. + * + * Returns false when there is nothing to close — a blank draft (already the + * post-close state) or a full-page view (skills / artifacts, which isn't a + * chat and owns no tab). ⌘W then stays a no-op; it never closes the window. + * + * `loadSessionIntoWorkspace` carries the app's route-based "load this session + * into main"; omitting it disables the promotion half. + */ +export function closeWorkspaceTab(loadSessionIntoWorkspace?: (storedSessionId: string) => void): boolean { + // Order matters — close the tile FIRST so the selection homes to the + // workspace instead of re-fronting the tile. + if (loadSessionIntoWorkspace) { + const next = nextSessionTileForWorkspace() + + if (next) { + closeSessionTile(next) + loadSessionIntoWorkspace(next) + + return true + } + } + + if ($workspaceIsPage.get() || !mainChatOccupied($activeSessionId.get(), $selectedStoredSessionId.get())) { + return false + } + + requestFreshSession() + + return true +} + /** * ⌘W — close the tab of the context you're in, by precedence: * 1. a focused terminal → its active terminal tab, * 2. right-rail tabs (live preview and/or file peeks), * 3. the FOCUSED chat zone → its active tab (a session tile stacked into it). - * 4. the workspace tab itself, when session tabs are stacked with it: - * the workspace can't close, so ⌘W shifts the NEXT session tab into main - * (loads it as the primary + drops its now-redundant tile). + * 4. the workspace tab itself — see `closeWorkspaceTab`. * Returns false when nothing closes, so ⌘W is a no-op — it never closes the - * window (a bare workspace stays put). Shared by the keyboard path (Win/Linux) - * and the macOS menu-accelerator IPC. + * window. Shared by the keyboard path (Win/Linux) and the macOS + * menu-accelerator IPC. * * Steps 3-4 follow the same focused zone ⌘1…⌘9 indexes, so a second chat zone * with its own tab strip closes ITS tab instead of main's. - * - * `loadSessionIntoWorkspace` carries the app's route-based "load this session - * into main" (the two call sites have router access); omitting it disables the - * step-4 promotion (⌘W stays the pre-existing no-op on the main tab). */ export function closeActiveTab(loadSessionIntoWorkspace?: (storedSessionId: string) => void): boolean { if (isFocusWithin('[data-terminal]')) { @@ -43,21 +84,5 @@ export function closeActiveTab(loadSessionIntoWorkspace?: (storedSessionId: stri return true } - // The main (workspace) tab is active and can't be closed — but if session - // tabs are stacked with it, ⌘W shifts the next one into the main tab: drop - // its tile (the session stays alive, no busy-close prompt) and load it into - // main. Order matters — close the tile FIRST so the selection homes to the - // workspace instead of re-fronting the tile. - if (loadSessionIntoWorkspace) { - const next = nextSessionTileForWorkspace() - - if (next) { - closeSessionTile(next) - loadSessionIntoWorkspace(next) - - return true - } - } - - return false + return closeWorkspaceTab(loadSessionIntoWorkspace) }