From ecf8ef970ee137e55c1fcf4028ca9affef767db9 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Sun, 26 Jul 2026 05:26:26 -0500 Subject: [PATCH] fix(desktop): clicking the active session from a full page returns to the chat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While the workspace pane shows a full page (Artifacts, Skills, Messaging, a plugin route), a sidebar click on the ACTIVE session did nothing: onResumeSession took focusOpenSession's `true` for the main-session branch as "already on screen" and skipped the navigate, but fronting the workspace tab doesn't put the chat back — the page is still routed. The user had to click some other session and then the active one to get back. focusOpenSession now reports WHICH surface it fronted ('main' | 'tile' | null), and focusedSessionNeedsRoute decides: a tile never needs a route (its pane renders the chat regardless), a main hit does while a page covers the workspace. --- apps/desktop/src/app/contrib/wiring.tsx | 18 ++++++++++--- apps/desktop/src/store/session-states.test.ts | 22 +++++++++++++++- apps/desktop/src/store/session-states.ts | 26 ++++++++++++++----- 3 files changed, 54 insertions(+), 12 deletions(-) diff --git a/apps/desktop/src/app/contrib/wiring.tsx b/apps/desktop/src/app/contrib/wiring.tsx index d00bd646c1cf..8398071f1c80 100644 --- a/apps/desktop/src/app/contrib/wiring.tsx +++ b/apps/desktop/src/app/contrib/wiring.tsx @@ -55,7 +55,7 @@ import { setCurrentProvider, setMessages } from '@/store/session' -import { focusOpenSession } from '@/store/session-states' +import { focusedSessionNeedsRoute, focusOpenSession } from '@/store/session-states' import { clearSessionTodos, setSessionTodos, todosForHydration } from '@/store/todos' import { isSecondaryWindow } from '@/store/windows' import { useSkinCommand } from '@/themes/use-skin-command' @@ -74,7 +74,14 @@ import { RemoteFolderPicker } from '../right-sidebar/files/remote-picker' import { resetProjectTreeState } from '../right-sidebar/files/use-project-tree' import { PersistentTerminal } from '../right-sidebar/terminal/persistent' import { closeAllTerminals } from '../right-sidebar/terminal/terminals' -import { CRON_ROUTE, routeSessionId, sessionRoute, SETTINGS_ROUTE, syncWorkspaceIsPage } from '../routes' +import { + $workspaceIsPage, + CRON_ROUTE, + routeSessionId, + sessionRoute, + SETTINGS_ROUTE, + syncWorkspaceIsPage +} from '../routes' import { SessionPickerOverlay } from '../session-picker-overlay' import { SessionSwitcher } from '../session-switcher' import { useBackgroundQueueDrain } from '../session/hooks/use-background-queue-drain' @@ -801,9 +808,12 @@ export function ContribWiring({ children }: { children: ReactNode }) { onRemoveAttachment: id => void composer.removeAttachment(id), onRestoreToMessage: restoreToMessage, // Already on screen (open tile, or the main session)? Jump to its tab; - // otherwise load it into main. + // otherwise load it into main. From a full page (artifacts, skills, …) a + // `'main'` hit still has to route back: fronting the workspace tab alone + // leaves the page showing, so clicking the ACTIVE session was a no-op and + // the user had to bounce off another row to get back to the chat. onResumeSession: sessionId => { - if (!focusOpenSession(sessionId)) { + if (focusedSessionNeedsRoute(focusOpenSession(sessionId), $workspaceIsPage.get())) { navigate(sessionRoute(sessionId)) } }, diff --git a/apps/desktop/src/store/session-states.test.ts b/apps/desktop/src/store/session-states.test.ts index 682ebe423d34..18d4142f1041 100644 --- a/apps/desktop/src/store/session-states.test.ts +++ b/apps/desktop/src/store/session-states.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from 'vitest' import { group, split } from '@/components/pane-shell/tree/model' import type { SessionTile } from '@/store/session-states' -import { orderTilesByTree, selectionHomesToWorkspace } from '@/store/session-states' +import { focusedSessionNeedsRoute, orderTilesByTree, selectionHomesToWorkspace } from '@/store/session-states' const tile = (storedSessionId: string): SessionTile => ({ storedSessionId }) const tilePane = (id: string) => `session-tile:${id}` @@ -44,3 +44,23 @@ describe('selectionHomesToWorkspace', () => { expect(selectionHomesToWorkspace('a', tiles)).toBe(false) }) }) + +describe('focusedSessionNeedsRoute', () => { + it('routes when the session is not on screen', () => { + expect(focusedSessionNeedsRoute(null, false)).toBe(true) + expect(focusedSessionNeedsRoute(null, true)).toBe(true) + }) + + it('routes for the ACTIVE main session while a full page covers the workspace', () => { + expect(focusedSessionNeedsRoute('main', true)).toBe(true) + }) + + it('skips the route when the main session is already the visible chat', () => { + expect(focusedSessionNeedsRoute('main', false)).toBe(false) + }) + + it('never routes for a tile — its pane shows the chat on any route', () => { + expect(focusedSessionNeedsRoute('tile', true)).toBe(false) + expect(focusedSessionNeedsRoute('tile', false)).toBe(false) + }) +}) diff --git a/apps/desktop/src/store/session-states.ts b/apps/desktop/src/store/session-states.ts index d8248f00d8ea..949d56e140ff 100644 --- a/apps/desktop/src/store/session-states.ts +++ b/apps/desktop/src/store/session-states.ts @@ -572,11 +572,14 @@ export function nextSessionTileForWorkspace(): null | string { } /** If a session is already ON SCREEN — an open tile OR the one loaded in main — - * front its tab (and focus its zone) and return true. A sidebar click on an - * already-open chat JUMPS to its tab instead of reloading it; `false` means the + * front its tab (and focus its zone) and report WHICH. A sidebar click on an + * already-open chat JUMPS to its tab instead of reloading it; `null` means the * caller must load it into main. Covers the two dead clicks: an open tile, and - * the main session while focus sits on a tile (route unchanged → no reload). */ -export function focusOpenSession(storedSessionId: string): boolean { + * the main session while focus sits on a tile (route unchanged → no reload). + * Callers that own the router need the `'main'` vs `'tile'` distinction: a + * `'main'` hit only reaches the screen if the workspace pane is actually + * showing the chat, whereas a tile renders in its own pane regardless. */ +export function focusOpenSession(storedSessionId: string): 'main' | 'tile' | null { if ($sessionTiles.get().some(t => t.storedSessionId === storedSessionId)) { const paneId = `${TILE_PANE_PREFIX}${storedSessionId}` revealTreePane(paneId) // un-dismiss + adopt + front in its group @@ -587,7 +590,7 @@ export function focusOpenSession(storedSessionId: string): boolean { noteActiveTreeGroup(group.id) } - return true + return 'tile' } // Already the main session: front the workspace tab and drop tile focus so @@ -596,10 +599,19 @@ export function focusOpenSession(storedSessionId: string): boolean { revealTreePane('workspace') noteActiveTreeGroup(null) - return true + return 'main' } - return false + return null +} + +/** Does a sidebar click still need to navigate after `focusOpenSession`? A miss + * always does. A `'main'` hit does too while the workspace pane is showing a + * full page (artifacts, skills, …): fronting the workspace tab doesn't put the + * chat back on screen — only a route change back to the session does. A tile + * hit never does; its pane renders the chat regardless of the route. */ +export function focusedSessionNeedsRoute(focused: 'main' | 'tile' | null, workspaceIsPage: boolean): boolean { + return !focused || (focused === 'main' && workspaceIsPage) } // Closed-tab stack for ⌘⇧T reopen (in-memory) — keyed PER PROFILE like the