From e67e4604dba174e1dd150ed3a36d4840dd597c87 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Tue, 28 Jul 2026 01:07:40 -0500 Subject: [PATCH] desktop: shared openSession door for focus-or-route Centralize open-in-place / tab / window so every surface can jump to an already-open tile or main tab instead of forcing main. --- apps/desktop/src/app/open-session.test.ts | 116 ++++++++++++++++++++++ apps/desktop/src/app/open-session.ts | 94 ++++++++++++++++++ 2 files changed, 210 insertions(+) create mode 100644 apps/desktop/src/app/open-session.test.ts create mode 100644 apps/desktop/src/app/open-session.ts diff --git a/apps/desktop/src/app/open-session.test.ts b/apps/desktop/src/app/open-session.test.ts new file mode 100644 index 00000000000..5345cd8b24b --- /dev/null +++ b/apps/desktop/src/app/open-session.test.ts @@ -0,0 +1,116 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const focusOpenSession = vi.fn() +const openSessionTile = vi.fn() +const openSessionInNewWindow = vi.fn() +const canOpenSessionWindow = vi.fn(() => true) +const workspaceIsPageGet = vi.fn(() => false) + +vi.mock('@/store/session-states', () => ({ + focusedSessionNeedsRoute: (focused: 'main' | 'tile' | null, workspaceIsPage: boolean) => + !focused || (focused === 'main' && workspaceIsPage), + focusOpenSession: (...args: unknown[]) => focusOpenSession(...args), + openSessionTile: (...args: unknown[]) => openSessionTile(...args) +})) + +vi.mock('@/store/windows', () => ({ + canOpenSessionWindow: () => canOpenSessionWindow(), + openSessionInNewWindow: (...args: unknown[]) => openSessionInNewWindow(...args) +})) + +vi.mock('./routes', () => ({ + $workspaceIsPage: { get: () => workspaceIsPageGet() }, + sessionRoute: (id: string) => `/c/${encodeURIComponent(id)}` +})) + +import { openSession, openSessionIntentFromModifiers } from './open-session' + +describe('openSessionIntentFromModifiers', () => { + it('defaults to in-place', () => { + expect(openSessionIntentFromModifiers()).toBe('in-place') + expect(openSessionIntentFromModifiers(null)).toBe('in-place') + expect(openSessionIntentFromModifiers({})).toBe('in-place') + }) + + it('reads ⌘/⌃ as tab and ⇧+mod as window', () => { + expect(openSessionIntentFromModifiers({ metaKey: true })).toBe('tab') + expect(openSessionIntentFromModifiers({ ctrlKey: true })).toBe('tab') + expect(openSessionIntentFromModifiers({ metaKey: true, shiftKey: true })).toBe('window') + expect(openSessionIntentFromModifiers({ shiftKey: true })).toBe('in-place') + }) +}) + +describe('openSession', () => { + const navigate = vi.fn() + + beforeEach(() => { + navigate.mockClear() + focusOpenSession.mockReset() + openSessionTile.mockReset() + openSessionInNewWindow.mockReset() + canOpenSessionWindow.mockReturnValue(true) + workspaceIsPageGet.mockReturnValue(false) + }) + + it('in-place focuses an existing tile and does not navigate', () => { + focusOpenSession.mockReturnValue('tile') + openSession('s1', navigate) + expect(focusOpenSession).toHaveBeenCalledWith('s1') + expect(navigate).not.toHaveBeenCalled() + expect(openSessionTile).not.toHaveBeenCalled() + }) + + it('in-place focuses main when already selected and not on a page', () => { + focusOpenSession.mockReturnValue('main') + openSession('s1', navigate) + expect(navigate).not.toHaveBeenCalled() + }) + + it('in-place routes when the main session is covered by a page', () => { + focusOpenSession.mockReturnValue('main') + workspaceIsPageGet.mockReturnValue(true) + openSession('s1', navigate) + expect(navigate).toHaveBeenCalledWith('/c/s1') + }) + + it('in-place routes when the session is not on screen', () => { + focusOpenSession.mockReturnValue(null) + openSession('s1', navigate) + expect(navigate).toHaveBeenCalledWith('/c/s1') + }) + + it('tab focuses an existing open session instead of stacking another', () => { + focusOpenSession.mockReturnValue('tile') + openSession('s1', navigate, 'tab') + expect(focusOpenSession).toHaveBeenCalledWith('s1') + expect(openSessionTile).not.toHaveBeenCalled() + expect(navigate).not.toHaveBeenCalled() + }) + + it('tab opens a stacked session tile when not on screen', () => { + focusOpenSession.mockReturnValue(null) + openSession('s1', navigate, 'tab') + expect(openSessionTile).toHaveBeenCalledWith('s1', 'center') + expect(navigate).not.toHaveBeenCalled() + }) + + it('window pops out when the bridge supports it', () => { + openSession('s1', navigate, 'window') + expect(openSessionInNewWindow).toHaveBeenCalledWith('s1') + expect(openSessionTile).not.toHaveBeenCalled() + }) + + it('window falls back to a tab when pop-out is unavailable', () => { + canOpenSessionWindow.mockReturnValue(false) + focusOpenSession.mockReturnValue(null) + openSession('s1', navigate, 'window') + expect(openSessionInNewWindow).not.toHaveBeenCalled() + expect(openSessionTile).toHaveBeenCalledWith('s1', 'center') + }) + + it('no-ops on an empty id', () => { + openSession('', navigate) + expect(navigate).not.toHaveBeenCalled() + expect(focusOpenSession).not.toHaveBeenCalled() + }) +}) diff --git a/apps/desktop/src/app/open-session.ts b/apps/desktop/src/app/open-session.ts new file mode 100644 index 00000000000..a9781b63535 --- /dev/null +++ b/apps/desktop/src/app/open-session.ts @@ -0,0 +1,94 @@ +/** + * One door for "open this session" — every surface (sidebar, ⌘K, notifications, + * session switcher, refs, cron/artifacts) goes through here so a chat that's + * already a tile (or the main tab) is JUMPED TO instead of yanked into main. + * + * Intents: + * - `in-place` (click / Enter) — focus existing tile/main if on screen; else + * load into main (same as the left sessions sidebar). + * - `tab` (⌘/⌃-click / ⌘-Enter / session refs) — focus if already on screen, + * else open as a stacked session tab (never steals main from under you). + * - `window` (⇧⌘-click) — pop into its own window; falls back to `tab` when + * the bridge has no session-window support. + */ +import { + focusedSessionNeedsRoute, + focusOpenSession, + openSessionTile +} from '@/store/session-states' +import { canOpenSessionWindow, openSessionInNewWindow } from '@/store/windows' + +import { $workspaceIsPage, sessionRoute } from './routes' + +export type OpenSessionIntent = 'in-place' | 'tab' | 'window' + +export type OpenSessionNavigate = (to: string, options?: { replace?: boolean }) => void + +/** Read modifiers the way session rows do — meta OR ctrl for tab, +shift for window. */ +export function openSessionIntentFromModifiers( + event?: null | { ctrlKey?: boolean; metaKey?: boolean; shiftKey?: boolean } +): OpenSessionIntent { + if (!event) { + return 'in-place' + } + + const mod = Boolean(event.metaKey || event.ctrlKey) + + if (mod && event.shiftKey) { + return 'window' + } + + if (mod) { + return 'tab' + } + + return 'in-place' +} + +/** + * @param navigate Required for `in-place` (route into main when not on screen). + * `tab` / `window` ignore it — pass a no-op when you don't have a router handle. + */ +export function openSession( + storedSessionId: string, + navigate: OpenSessionNavigate, + intent: OpenSessionIntent = 'in-place' +): void { + if (!storedSessionId) { + return + } + + let resolved: OpenSessionIntent = intent + + if (resolved === 'window') { + if (canOpenSessionWindow()) { + void openSessionInNewWindow(storedSessionId) + + return + } + + // No pop-out support → treat like a new tab. + resolved = 'tab' + } + + if (resolved === 'tab') { + // Already on screen? Front it. openSessionTile would no-op on main without + // focusing, or try to relocate an existing tile — neither is right for a + // soft "open beside" link. + if (focusOpenSession(storedSessionId)) { + return + } + + openSessionTile(storedSessionId, 'center') + + return + } + + // Already on screen (open tile, or the main session)? Jump to its tab; + // 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. + if (focusedSessionNeedsRoute(focusOpenSession(storedSessionId), $workspaceIsPage.get())) { + navigate(sessionRoute(storedSessionId)) + } +}