diff --git a/apps/desktop/src/app/chat/sidebar/session-actions-menu.test.ts b/apps/desktop/src/app/chat/sidebar/session-actions-menu.test.ts new file mode 100644 index 00000000000..321300ee8d3 --- /dev/null +++ b/apps/desktop/src/app/chat/sidebar/session-actions-menu.test.ts @@ -0,0 +1,92 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' + +import { $activeSessionId, $selectedStoredSessionId } from '@/store/session' + +import { renameSessionPreferringRpc } from './session-actions-menu' + +// The branched-session rename bug: a freshly branched session lives only in the +// gateway's runtime _sessions map (no state.db row yet), so REST PATCH +// /api/sessions/{id} 404s with "Session not found". renameSessionPreferringRpc +// must route the ACTIVE row through the session.title RPC (runtime id), which +// persists the row on demand, and otherwise fall back to REST. + +const renameSession = vi.fn(async () => ({ ok: true, title: 'rest-title' })) +const request = vi.fn(async () => ({ title: 'rpc-title' }) as never) +const activeGateway = vi.fn<() => { request: typeof request } | null>(() => ({ request })) + +vi.mock('@/hermes', () => ({ + renameSession: (...args: unknown[]) => renameSession(...(args as [])), + HermesGateway: class {} +})) + +vi.mock('@/store/gateway', () => ({ + activeGateway: () => activeGateway() +})) + +const RUNTIME_ID = 'rt-runtime-1' +const STORED_ID = 'stored-branch-1' + +afterEach(() => { + renameSession.mockClear() + request.mockClear() + activeGateway.mockReset() + activeGateway.mockReturnValue({ request }) + $activeSessionId.set(null) + $selectedStoredSessionId.set(null) +}) + +describe('renameSessionPreferringRpc', () => { + it('renames the active branched session via the session.title RPC, not REST', async () => { + $selectedStoredSessionId.set(STORED_ID) + $activeSessionId.set(RUNTIME_ID) + + const result = await renameSessionPreferringRpc(STORED_ID, 'My branch') + + expect(request).toHaveBeenCalledWith('session.title', { session_id: RUNTIME_ID, title: 'My branch' }) + expect(renameSession).not.toHaveBeenCalled() + expect(result.title).toBe('rpc-title') + }) + + it('falls back to REST when the RPC fails (e.g. socket mid-reconnect)', async () => { + $selectedStoredSessionId.set(STORED_ID) + $activeSessionId.set(RUNTIME_ID) + request.mockRejectedValueOnce(new Error('not connected')) + + const result = await renameSessionPreferringRpc(STORED_ID, 'My branch', 'work') + + expect(request).toHaveBeenCalledOnce() + expect(renameSession).toHaveBeenCalledWith(STORED_ID, 'My branch', 'work') + expect(result.title).toBe('rest-title') + }) + + it('uses REST for a non-active row (background/persisted session)', async () => { + $selectedStoredSessionId.set('some-other-active-session') + $activeSessionId.set(RUNTIME_ID) + + await renameSessionPreferringRpc(STORED_ID, 'My branch', 'work') + + expect(request).not.toHaveBeenCalled() + expect(renameSession).toHaveBeenCalledWith(STORED_ID, 'My branch', 'work') + }) + + it('uses REST when clearing the title (RPC rejects empty titles)', async () => { + $selectedStoredSessionId.set(STORED_ID) + $activeSessionId.set(RUNTIME_ID) + + await renameSessionPreferringRpc(STORED_ID, '') + + expect(request).not.toHaveBeenCalled() + expect(renameSession).toHaveBeenCalledWith(STORED_ID, '', undefined) + }) + + it('uses REST when no gateway is connected', async () => { + $selectedStoredSessionId.set(STORED_ID) + $activeSessionId.set(RUNTIME_ID) + activeGateway.mockReturnValue(null) + + await renameSessionPreferringRpc(STORED_ID, 'My branch') + + expect(request).not.toHaveBeenCalled() + expect(renameSession).toHaveBeenCalledWith(STORED_ID, 'My branch', undefined) + }) +}) diff --git a/apps/desktop/src/app/chat/sidebar/session-actions-menu.tsx b/apps/desktop/src/app/chat/sidebar/session-actions-menu.tsx index abff74dcfc5..4453097c044 100644 --- a/apps/desktop/src/app/chat/sidebar/session-actions-menu.tsx +++ b/apps/desktop/src/app/chat/sidebar/session-actions-menu.tsx @@ -19,10 +19,58 @@ import { renameSession } from '@/hermes' import { useI18n } from '@/i18n' import { triggerHaptic } from '@/lib/haptics' import { exportSession } from '@/lib/session-export' +import { activeGateway } from '@/store/gateway' import { notify, notifyError } from '@/store/notifications' -import { setSessions } from '@/store/session' +import { $activeSessionId, $selectedStoredSessionId, setSessions } from '@/store/session' import { canOpenSessionWindow, openSessionInNewWindow } from '@/store/windows' +import type { SessionTitleResponse } from '../../types' + +// Rename a session, preferring the gateway's session.title RPC over REST. +// +// A freshly *branched* session (and any brand-new chat) lives only in the +// gateway's in-memory _sessions map keyed by its RUNTIME id — no row is +// persisted to state.db until the first turn. REST PATCH /api/sessions/{id} +// resolves against the stored sessions table, so it 404s ("Session not found") +// on these runtime-only sessions. The session.title RPC resolves the live +// runtime session AND persists the row on demand, so it succeeds where REST +// cannot. This mirrors the /title slash command's fix (use-prompt-actions.ts). +// +// We only take the RPC path for the ACTIVE/selected session: its runtime id is +// known ($activeSessionId) and it lives on the active gateway, so there is no +// profile-routing ambiguity. Every other row (already persisted, possibly on a +// background profile) keeps the REST path, which handles profile scoping and a +// non-empty title is required by the RPC (it rejects clears), so clears stay on +// REST too. +export async function renameSessionPreferringRpc( + storedSessionId: string, + title: string, + profile?: string +): Promise<{ title?: string }> { + const isActiveRow = storedSessionId === $selectedStoredSessionId.get() + const runtimeId = isActiveRow ? $activeSessionId.get() : null + const gateway = activeGateway() + + if (title && runtimeId && gateway) { + try { + const result = await gateway.request('session.title', { + session_id: runtimeId, + title + }) + + return { title: result?.title ?? title } + } catch (err) { + // Fall through to REST — e.g. the socket is mid-reconnect. REST still + // works for any session that already has a persisted row. Log so a + // genuine RPC-side failure (which then surfaces a REST 404 for the + // runtime id) is at least diagnosable instead of silently swallowed. + console.warn('session.title RPC rename failed; falling back to REST', err) + } + } + + return renameSession(storedSessionId, title, profile) +} + interface SessionActions { sessionId: string title: string @@ -235,7 +283,7 @@ function RenameSessionDialog({ open, onOpenChange, sessionId, currentTitle, prof setSubmitting(true) try { - const result = await renameSession(sessionId, next, profile) + const result = await renameSessionPreferringRpc(sessionId, next, profile) const finalTitle = result.title || next || '' setSessions(prev => prev.map(s => (s.id === sessionId ? { ...s, title: finalTitle || null } : s))) notify({ durationMs: 2_000, kind: 'success', message: r.renamed })