Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions apps/desktop/src/app/command-palette/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -451,11 +451,13 @@ export function CommandPalette() {

const go = useCallback((path: string) => () => navigateToWorkspacePage(navigate, path), [navigate])

// Sessions: plain select = open in-place (focus existing tile/main, else main);
// ⌘/⌃-select / ⌘-Enter = new tab; ⇧⌘ = own window. Same door as the sidebar.
// Sessions: plain select = open beside what's already loaded (focus existing
// tile/main, else a new tab — main only when it's a blank draft);
// ⌘/⌃-select / ⌘-Enter = force a new tab; ⇧⌘ = own window. Same door as the
// sidebar, minus the sidebar's licence to spend main.
const goSession = useCallback(
(sessionId: string) => (event?: { ctrlKey?: boolean; metaKey?: boolean; shiftKey?: boolean }) => {
openSession(sessionId, navigate, openSessionIntentFromModifiers(event))
openSession(sessionId, navigate, openSessionIntentFromModifiers(event, 'stack'))
},
[navigate]
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,14 @@ export function useDesktopIntegrations({
}, [resumeExhaustedSessionId])

// Native-notification click -> jump to the session WHERE IT ALREADY IS (open
// tile / main) instead of forcing main. Runtime id is translated to the
// stored id the chat route is keyed by; action buttons resolve in place.
// tile / main), else beside what's loaded rather than over it — the click
// came from outside the app and shouldn't cost the user the chat they left
// on screen. Runtime id is translated to the stored id the chat route is
// keyed by; action buttons resolve in place.
useEffect(() => {
const unsubscribe = window.hermesDesktop?.onFocusSession?.(sessionId => {
if (sessionId) {
openSession(storedSessionIdForNotification(sessionId, runtimeIdByStoredSessionId.current), navigate)
openSession(storedSessionIdForNotification(sessionId, runtimeIdByStoredSessionId.current), navigate, 'stack')
}
})

Expand Down
8 changes: 4 additions & 4 deletions apps/desktop/src/app/contrib/wiring.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ import { useGatewayRequest } from '../gateway/hooks/use-gateway-request'
import { useKeybinds } from '../hooks/use-keybinds'
import { ModelPickerOverlay } from '../model-picker-overlay'
import { ModelVisibilityOverlay } from '../model-visibility-overlay'
import { openSession } from '../open-session'
import { mainChatOccupied, openSession } from '../open-session'
import { PetGenerateOverlay } from '../pet-generate/pet-generate-overlay'
import { FileActionDialogs } from '../right-sidebar/file-actions'
import { RemoteFolderPicker } from '../right-sidebar/files/remote-picker'
Expand All @@ -98,7 +98,7 @@ import { useRouteResume } from '../session/hooks/use-route-resume'
import { useSessionActions } from '../session/hooks/use-session-actions'
import { useSessionListActions } from '../session/hooks/use-session-list-actions'
import { useSessionStateCache } from '../session/hooks/use-session-state-cache'
import { newSessionOpensTab, startWorkspaceSession } from '../session/workspace-session-target'
import { startWorkspaceSession } from '../session/workspace-session-target'
import { useOverlayRouting } from '../shell/hooks/use-overlay-routing'
import { useWindowControlsOverlayWidth } from '../shell/hooks/use-window-controls-overlay-width'
import { titlebarControlsPosition } from '../shell/titlebar'
Expand Down Expand Up @@ -493,12 +493,12 @@ export function ContribWiring({ children }: { children: ReactNode }) {
// project so the new lane is visible.
//
// `openTab` is the sidebar "+" behavior: once a chat is loaded, stack a new
// tab instead of replacing it (see newSessionOpensTab). The composer's
// tab instead of replacing it (see mainChatOccupied). The composer's
// "branch off into a new worktree" flow keeps the fresh-draft path — it
// prefills the MAIN composer right after, so it has to own that surface.
const startSessionInWorkspace = useCallback(
(path: null | string, options?: { openTab?: boolean }) => {
if (options?.openTab && newSessionOpensTab(activeSessionIdRef.current, $selectedStoredSessionId.get())) {
if (options?.openTab && mainChatOccupied(activeSessionIdRef.current, $selectedStoredSessionId.get())) {
void openNewSessionTile('center', { cwd: path, listed: false })

return
Expand Down
94 changes: 92 additions & 2 deletions apps/desktop/src/app/open-session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'

const focusOpenSession = vi.fn()
const openSessionTile = vi.fn()
const reuseBlankDraftTile = vi.fn()
const openSessionInNewWindow = vi.fn()
const canOpenSessionWindow = vi.fn(() => true)
const workspaceIsPageGet = vi.fn(() => false)
Expand All @@ -10,7 +11,8 @@ 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)
openSessionTile: (...args: unknown[]) => openSessionTile(...args),
reuseBlankDraftTile: (...args: unknown[]) => reuseBlankDraftTile(...args)
}))

vi.mock('@/store/windows', () => ({
Expand All @@ -23,7 +25,33 @@ vi.mock('./routes', () => ({
sessionRoute: (id: string) => `/c/${encodeURIComponent(id)}`
}))

import { openSession, openSessionIntentFromModifiers } from './open-session'
import { $activeSessionId, $selectedStoredSessionId } from '@/store/session'

import { mainChatOccupied, openSession, openSessionIntentFromModifiers } from './open-session'

/**
* The question behind both the sidebar "+" and a palette open: is there a
* conversation on main that must not be discarded? A create affordance stacks a
* tab rather than replacing a chat that may still be mid-turn, and an open from
* nowhere does the same.
*/
describe('mainChatOccupied', () => {
it('is occupied once a conversation is on screen', () => {
expect(mainChatOccupied('runtime-a', 'stored-a')).toBe(true)
})

it('is occupied by a live runtime whose stored id has not landed yet', () => {
expect(mainChatOccupied('runtime-a', null)).toBe(true)
})

it('is occupied by a selected session still resuming into a runtime', () => {
expect(mainChatOccupied(null, 'stored-a')).toBe(true)
})

it('is free when nothing is open', () => {
expect(mainChatOccupied(null, null)).toBe(false)
})
})

describe('openSessionIntentFromModifiers', () => {
it('defaults to in-place', () => {
Expand All @@ -32,12 +60,22 @@ describe('openSessionIntentFromModifiers', () => {
expect(openSessionIntentFromModifiers({})).toBe('in-place')
})

it('returns the caller base for an unmodified select', () => {
expect(openSessionIntentFromModifiers(undefined, 'stack')).toBe('stack')
expect(openSessionIntentFromModifiers({}, 'stack')).toBe('stack')
})

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')
})

it('lets modifiers override the base', () => {
expect(openSessionIntentFromModifiers({ metaKey: true }, 'stack')).toBe('tab')
expect(openSessionIntentFromModifiers({ metaKey: true, shiftKey: true }, 'stack')).toBe('window')
})
})

describe('openSession', () => {
Expand All @@ -50,6 +88,9 @@ describe('openSession', () => {
openSessionInNewWindow.mockReset()
canOpenSessionWindow.mockReturnValue(true)
workspaceIsPageGet.mockReturnValue(false)
reuseBlankDraftTile.mockReset()
$activeSessionId.set(null)
$selectedStoredSessionId.set(null)
})

it('in-place focuses an existing tile and does not navigate', () => {
Expand Down Expand Up @@ -94,6 +135,55 @@ describe('openSession', () => {
expect(navigate).not.toHaveBeenCalled()
})

it('stack focuses a session that is already on screen', () => {
$selectedStoredSessionId.set('s0')
focusOpenSession.mockReturnValue('tile')
openSession('s1', navigate, 'stack')
expect(openSessionTile).not.toHaveBeenCalled()
expect(navigate).not.toHaveBeenCalled()
})

it('stack opens a tab rather than taking main from a loaded chat', () => {
$selectedStoredSessionId.set('s0')
focusOpenSession.mockReturnValue(null)
openSession('s1', navigate, 'stack')
expect(openSessionTile).toHaveBeenCalledWith('s1', 'center')
expect(navigate).not.toHaveBeenCalled()
})

it('stack spends an open blank draft tab before stacking a new one', () => {
$selectedStoredSessionId.set('s0')
focusOpenSession.mockReturnValue(null)
reuseBlankDraftTile.mockReturnValue(true)
openSession('s1', navigate, 'stack')
expect(reuseBlankDraftTile).toHaveBeenCalledWith('s1')
expect(openSessionTile).not.toHaveBeenCalled()
expect(navigate).not.toHaveBeenCalled()
})

it('stack prefers the session already on screen over a blank draft tab', () => {
$selectedStoredSessionId.set('s0')
focusOpenSession.mockReturnValue('tile')
reuseBlankDraftTile.mockReturnValue(true)
openSession('s1', navigate, 'stack')
expect(reuseBlankDraftTile).not.toHaveBeenCalled()
expect(openSessionTile).not.toHaveBeenCalled()
})

it('stack opens a tab while main is mid-turn on an unsaved session', () => {
$activeSessionId.set('runtime-a')
focusOpenSession.mockReturnValue(null)
openSession('s1', navigate, 'stack')
expect(openSessionTile).toHaveBeenCalledWith('s1', 'center')
})

it('stack loads into main when it holds only a blank draft', () => {
focusOpenSession.mockReturnValue(null)
openSession('s1', navigate, 'stack')
expect(navigate).toHaveBeenCalledWith('/c/s1')
expect(openSessionTile).not.toHaveBeenCalled()
})

it('window pops out when the bridge supports it', () => {
openSession('s1', navigate, 'window')
expect(openSessionInNewWindow).toHaveBeenCalledWith('s1')
Expand Down
58 changes: 50 additions & 8 deletions apps/desktop/src/app/open-session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,28 +4,52 @@
* 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).
* - `in-place` (sidebar click / Enter) — focus existing tile/main if on
* screen; else load into main (same as the left sessions sidebar).
* - `stack` (⌘K, notifications — anything that opens a chat from outside the
* workspace) — like `tab`, but may spend main or an open blank draft tab
* when either is empty.
* - `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 { $activeSessionId, $selectedStoredSessionId } from '@/store/session'
import {
focusedSessionNeedsRoute,
focusOpenSession,
openSessionTile,
reuseBlankDraftTile
} from '@/store/session-states'
import { canOpenSessionWindow, openSessionInNewWindow } from '@/store/windows'

import { $workspaceIsPage, sessionRoute } from './routes'

export type OpenSessionIntent = 'in-place' | 'tab' | 'window'
export type OpenSessionIntent = 'in-place' | 'stack' | '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. */
/**
* Is the main tab holding a conversation worth preserving?
*
* A loaded chat may still be mid-turn, so replacing it with something else
* throws away work the user can see. A blank draft has nothing to lose, which
* is what lets the sidebar "+" and a `stack` open take the cheaper main path
* instead of stacking a tab nobody asked for.
*/
export function mainChatOccupied(activeSessionId: null | string, selectedStoredSessionId: null | string): boolean {
return Boolean(activeSessionId || selectedStoredSessionId)
}

/** Read modifiers the way session rows do — meta OR ctrl for tab, +shift for
* window. `base` is what an unmodified select means for the caller: the
* sidebar spends main (`in-place`), a palette-style open doesn't (`stack`). */
export function openSessionIntentFromModifiers(
event?: null | { ctrlKey?: boolean; metaKey?: boolean; shiftKey?: boolean }
event?: null | { ctrlKey?: boolean; metaKey?: boolean; shiftKey?: boolean },
base: OpenSessionIntent = 'in-place'
): OpenSessionIntent {
if (!event) {
return 'in-place'
return base
}

const mod = Boolean(event.metaKey || event.ctrlKey)
Expand All @@ -38,7 +62,7 @@ export function openSessionIntentFromModifiers(
return 'tab'
}

return 'in-place'
return base
}

/**
Expand Down Expand Up @@ -67,6 +91,17 @@ export function openSession(
resolved = 'tab'
}

// A `stack` open arrives from outside the workspace, so unlike a sidebar
// click it can't assume main is spendable: it behaves like `tab`, except main
// IS fair game while it's only a blank draft, and an already-open blank draft
// tab is spent before a new one is stacked.
let spendBlankDraft = false

if (resolved === 'stack') {
spendBlankDraft = mainChatOccupied($activeSessionId.get(), $selectedStoredSessionId.get())
resolved = spendBlankDraft ? 'tab' : 'in-place'
}

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
Expand All @@ -75,6 +110,13 @@ export function openSession(
return
}

// Nothing to jump to, but an open tab may still be an empty "New session" —
// that's the tab the user would have typed into, so spend it rather than
// stacking a second blank one beside it.
if (spendBlankDraft && reuseBlankDraftTile(storedSessionId)) {
return
}

openSessionTile(storedSessionId, 'center')

return
Expand Down
25 changes: 1 addition & 24 deletions apps/desktop/src/app/session/workspace-session-target.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
setNewChatWorkspaceTarget
} from '@/store/session'

import { newSessionOpensTab, startWorkspaceSession } from './workspace-session-target'
import { startWorkspaceSession } from './workspace-session-target'

function deferred<T>() {
let resolve!: (value: T) => void
Expand All @@ -21,29 +21,6 @@ function deferred<T>() {
return { promise, resolve }
}

/**
* The sidebar "+" is a create affordance, not a discard one: with a chat
* already loaded it must stack a tab (⌘T) rather than replace the surface
* (⌘N), which would throw away a conversation that may still be mid-turn.
*/
describe('newSessionOpensTab', () => {
it('opens a tab once a conversation is on screen', () => {
expect(newSessionOpensTab('runtime-a', 'stored-a')).toBe(true)
})

it('opens a tab for a live runtime whose stored id has not landed yet', () => {
expect(newSessionOpensTab('runtime-a', null)).toBe(true)
})

it('opens a tab for a selected session still resuming into a runtime', () => {
expect(newSessionOpensTab(null, 'stored-a')).toBe(true)
})

it('replaces the empty surface when nothing is open', () => {
expect(newSessionOpensTab(null, null)).toBe(false)
})
})

describe('startWorkspaceSession', () => {
afterEach(() => {
setCurrentBranch('')
Expand Down
13 changes: 0 additions & 13 deletions apps/desktop/src/app/session/workspace-session-target.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,6 @@ interface WorkspaceSessionOptions {
startFreshSessionDraft: (options?: { workspaceTarget: string }) => void
}

/**
* Should the sidebar "+" open a NEW TAB rather than replace what's on screen?
*
* "+" is a create affordance, never a discard one. Once a conversation is
* loaded, replacing it with a blank draft would throw away a chat that may
* still be mid-turn, so the button stacks a tab beside it (⌘T) instead of
* taking over the surface (⌘N). With nothing open there is no tab worth
* preserving and the cheaper fresh-draft path applies.
*/
export function newSessionOpensTab(activeSessionId: null | string, selectedStoredSessionId: null | string): boolean {
return Boolean(activeSessionId || selectedStoredSessionId)
}

export function startWorkspaceSession({
activeSessionIdRef,
followActiveSessionCwd: followCwd = followActiveSessionCwd,
Expand Down
Loading
Loading