diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx index f27fc5b750a5..a7de88bf4de3 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx @@ -3,6 +3,7 @@ import type { MutableRefObject } from 'react' import { useEffect, useRef } from 'react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { getSession } from '@/hermes' import { textPart } from '@/lib/chat-messages' import { $composerAttachments, $composerDraft, type ComposerAttachment, setComposerDraft } from '@/store/composer' import { $notifications, clearNotifications } from '@/store/notifications' @@ -25,6 +26,7 @@ import { uploadComposerAttachment, usePromptActions } from '.' vi.mock('@/hermes', () => ({ getProfiles: vi.fn(async () => ({ profiles: [] })), + getSession: vi.fn(), PROMPT_SUBMIT_REQUEST_TIMEOUT_MS: 1_800_000, setApiRequestProfile: vi.fn(), transcribeAudio: vi.fn() @@ -1834,6 +1836,99 @@ describe('usePromptActions sleep/wake session recovery', () => { expect(calls[2]?.params).toEqual({ session_id: RECOVERED_SESSION_ID, text: 'message after wake' }) }) + // #67603 (second symptom): a recovery resume must re-register on the session's + // OWNING profile. Resuming on whichever profile is live forks the conversation + // into the wrong profile's DB — the session then appears under both profiles. + it('carries the owning profile from the cache into the recovery resume', async () => { + setSessions(() => [sessionInfo({ id: STORED_SESSION_ID, profile: 'work' })]) + + const calls: { method: string; params?: Record }[] = [] + let submitAttempts = 0 + + const requestGateway = vi.fn(async (method: string, params?: Record) => { + calls.push({ method, params }) + + if (method === 'prompt.submit') { + submitAttempts += 1 + + if (submitAttempts === 1) { + throw new Error('session not found') + } + + return {} as never + } + + if (method === 'session.resume') { + return { session_id: RECOVERED_SESSION_ID } as never + } + + return {} as never + }) + + let handle: HarnessHandle | null = null + await actRender( + (handle = h)} + refreshSessions={async () => undefined} + requestGateway={requestGateway} + storedSessionId={STORED_SESSION_ID} + /> + ) + + expect(await handle!.submitText('message after wake')).toBe(true) + expect(calls[1]?.params).toEqual({ session_id: STORED_SESSION_ID, source: 'desktop', profile: 'work' }) + + setSessions(() => []) + }) + + // The session lives on another profile and is outside the paginated sidebar + // cache: resolve it by id across profiles rather than resuming profile-blind. + it('resolves the owning profile across profiles when the session is not cached', async () => { + // module-factory vi.fn is not reset by restoreAllMocks — reset explicitly in + // the finally below so this resolved value never leaks into sibling tests. + setSessions(() => []) + vi.mocked(getSession).mockResolvedValue(sessionInfo({ id: STORED_SESSION_ID, profile: 'work' })) + + const calls: { method: string; params?: Record }[] = [] + let submitAttempts = 0 + + const requestGateway = vi.fn(async (method: string, params?: Record) => { + calls.push({ method, params }) + + if (method === 'prompt.submit') { + submitAttempts += 1 + + if (submitAttempts === 1) { + throw new Error('session not found') + } + + return {} as never + } + + if (method === 'session.resume') { + return { session_id: RECOVERED_SESSION_ID } as never + } + + return {} as never + }) + + let handle: HarnessHandle | null = null + await actRender( + (handle = h)} + refreshSessions={async () => undefined} + requestGateway={requestGateway} + storedSessionId={STORED_SESSION_ID} + /> + ) + + expect(await handle!.submitText('message after wake')).toBe(true) + expect(calls[1]?.params).toEqual({ session_id: STORED_SESSION_ID, source: 'desktop', profile: 'work' }) + + vi.mocked(getSession).mockReset() + setSessions(() => []) + }) + it('background queue resume uses the queued stored id and leaves foreground runtime selected', async () => { const calls: { method: string; params?: Record }[] = [] let submitAttempts = 0 diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts index f7edf24f14c1..1e314fe714a8 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts @@ -43,6 +43,7 @@ import type { ImageAttachResponse, SessionRedirectResponse } from '../../../types' +import { resolveSessionProfile } from '../use-session-actions/utils' import { applyBranchVisibility, @@ -601,9 +602,12 @@ export function usePromptActions({ if (isSessionNotFoundError(err) && selectedStoredSessionIdRef.current) { try { + const resumeProfile = await resolveSessionProfile(selectedStoredSessionIdRef.current) + const resumed = await requestGateway<{ session_id: string }>('session.resume', { session_id: selectedStoredSessionIdRef.current, - source: 'desktop' + source: 'desktop', + ...(resumeProfile ? { profile: resumeProfile } : {}) }) const recoveredId = resumed?.session_id @@ -700,9 +704,12 @@ export function usePromptActions({ // correction right after a reconnect isn't lost to the race. if (isSessionNotFoundError(err) && selectedStoredSessionIdRef.current) { try { + const resumeProfile = await resolveSessionProfile(selectedStoredSessionIdRef.current) + const resumed = await requestGateway<{ session_id: string }>('session.resume', { session_id: selectedStoredSessionIdRef.current, - source: 'desktop' + source: 'desktop', + ...(resumeProfile ? { profile: resumeProfile } : {}) }) const recoveredId = resumed?.session_id diff --git a/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts b/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts index deee5b0683ba..398a72f3be4a 100644 --- a/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts +++ b/apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts @@ -24,6 +24,7 @@ import { setAwaitingResponse, setBusy, setMessages } from '@/store/session' import type { ClientSessionState } from '../../../types' import { sessionContextDrift } from '../session-context-drift' +import { resolveSessionProfile } from '../use-session-actions/utils' import { _submitInFlight, @@ -384,9 +385,14 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) { // background queue drain only has the durable id). Continue that target // conversation; only a genuine new-chat draft may create a new session. try { + // Re-register on the session's OWNING profile — resuming on whichever + // profile is live would fork the conversation into the wrong DB (#67603). + const resumeProfile = await resolveSessionProfile(targetStoredSessionId) + const resumed = await requestGateway<{ session_id: string }>('session.resume', { session_id: targetStoredSessionId, - source: 'desktop' + source: 'desktop', + ...(resumeProfile ? { profile: resumeProfile } : {}) }) const resumeDrift = sessionDriftReason() @@ -528,9 +534,12 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) { // backend loop (#55578 symptom d) rejects the submit even though // the stored session is fine — resume + retry instead of erroring // out and losing the session binding. + const resumeProfile = await resolveSessionProfile(recoverStoredSessionId) + const resumed = await requestGateway<{ session_id: string }>('session.resume', { session_id: recoverStoredSessionId, - source: 'desktop' + source: 'desktop', + ...(resumeProfile ? { profile: resumeProfile } : {}) }) const resumeRetryDrift = sessionDriftReason() diff --git a/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx b/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx index 5188a11f8d26..f0a4139f7990 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx @@ -3,7 +3,7 @@ import type { MutableRefObject } from 'react' import { useEffect } from 'react' import { afterEach, describe, expect, it, vi } from 'vitest' -import { getSessionMessages, type SessionInfo } from '@/hermes' +import { getSession, getSessionMessages, type SessionInfo } from '@/hermes' import { createClientSessionState } from '@/lib/chat-runtime' import { clearSessionDraft, stashSessionDraft, takeSessionDraft } from '@/store/composer' import { $activeGatewayProfile, $newChatProfile, ensureGatewayProfile } from '@/store/profile' @@ -43,6 +43,7 @@ import { useSessionActions } from './use-session-actions' vi.mock('@/hermes', async importOriginal => ({ ...(await importOriginal>()), deleteSession: vi.fn(), + getSession: vi.fn(), getSessionMessages: vi.fn(), listAllProfileSessions: vi.fn(), setApiRequestProfile: vi.fn(), @@ -1088,6 +1089,39 @@ describe('branchStoredSession desktop source tagging', () => { source: 'desktop' }) }) + + // #67603: right-clicking a session outside the paginated sidebar window is a + // cache miss. Resolve its owning profile (cache → active → cross-profile) and + // swap to it before reading the transcript / creating the branch, so the fork + // is not created on whichever profile happens to be live. + it('resolves and swaps to the parent profile when the branched session is not cached', async () => { + setSessions([]) + vi.mocked(getSession).mockResolvedValue(storedSession({ id: 'stored-parent', message_count: 1, profile: 'work' })) + vi.mocked(getSessionMessages).mockResolvedValue({ + messages: [{ content: 'branch me', role: 'user', timestamp: 1 }], + session_id: 'stored-parent' + } as never) + + const requestGateway = vi.fn(async (method: string) => { + if (method === 'session.create') { + return { session_id: 'branch-runtime', stored_session_id: 'branch-stored' } as never + } + + return {} as never + }) + + let branchStoredSession: ((storedSessionId: string, sessionProfile?: string | null) => Promise) | null = + null + render( (branchStoredSession = branch)} requestGateway={requestGateway} />) + await waitFor(() => expect(branchStoredSession).not.toBeNull()) + + await expect(branchStoredSession!('stored-parent')).resolves.toBe(true) + + expect(ensureGatewayProfile).toHaveBeenCalledWith('work') + expect(getSessionMessages).toHaveBeenCalledWith('stored-parent', 'work') + + vi.mocked(getSession).mockReset() + }) }) // ── Warm-cache mapping integrity (the "open chat A, chat B loads" bug) ───────── diff --git a/apps/desktop/src/app/session/hooks/use-session-actions/index.ts b/apps/desktop/src/app/session/hooks/use-session-actions/index.ts index 75aaf8cd6cab..a4ac8ec4ad72 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions/index.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions/index.ts @@ -1177,7 +1177,13 @@ export function useSessionActions({ async (storedSessionId: string, sessionProfile?: string | null): Promise => { clearNotifications() - const stored = $sessions.get().find(session => sessionMatchesStoredId(session, storedSessionId)) + // Right-clicking a session outside the paginated sidebar window is a cache + // miss: resolve it (cache → active backend → cross-profile) so the branch + // is created on the parent's OWNING profile, not whichever is live (#67603). + const stored = + $sessions.get().find(session => sessionMatchesStoredId(session, storedSessionId)) ?? + (sessionProfile ? undefined : await resolveStoredSession(storedSessionId)) + const profile = sessionProfile ?? stored?.profile try { diff --git a/apps/desktop/src/app/session/hooks/use-session-actions/utils.test.ts b/apps/desktop/src/app/session/hooks/use-session-actions/utils.test.ts index d2fab21de6ca..2f0d7c05f5f9 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions/utils.test.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions/utils.test.ts @@ -361,6 +361,58 @@ describe('preserveLocalPendingTurnMessages', () => { expect(preserveLocalPendingTurnMessages(compressedAuthority, pollutedWarmCache)).toBe(compressedAuthority) }) + + // #67603: the gateway persists model-switch / personality notices as role=user + // ([System: …], tui_gateway/server.py). A single trailing marker is already + // handled by the latestAuthoritativeUser guard above, but TWO switches around + // one turn put a marker BEFORE the committed prompt (shifting its ordinal) and + // another AFTER it (so the prompt is no longer the last user row, so the text + // guard can't rescue it). Naive ordinal pairing then pairs the optimistic row + // against a marker, treats it as uncommitted, and re-appends it — the + // duplicated user bubble stacked at the bottom of the chat. + it('does not duplicate the optimistic prompt when markers bracket it (two model switches)', () => { + const marker = (name: string) => `[System: The active model for this chat has changed to ${name}.]` + + const previous = [ + msg('1-user', 'user', 'first'), + msg('2-assistant', 'assistant', 'first answer'), + msg('user-optimistic', 'user', 'second question') + ] + + const next = [ + msg('s1-user', 'user', 'first'), + msg('s2-assistant', 'assistant', 'first answer'), + msg('s3-marker', 'user', marker('k2')), + msg('s4-user', 'user', 'second question'), + msg('s5-assistant', 'assistant', 'second answer'), + msg('s6-marker', 'user', marker('k3')) + ] + + expect(preserveLocalPendingTurnMessages(next, previous)).toBe(next) + }) + + it('still keeps a genuinely uncommitted optimistic turn when a marker is present', () => { + const previous = [ + msg('1-user', 'user', 'first'), + msg('2-assistant', 'assistant', 'first answer'), + msg('user-optimistic', 'user', 'new question') + ] + + // The marker is persisted but the new prompt has not committed yet — the + // optimistic row must survive (marker exclusion must not over-correct). + const next = [ + msg('1-user-stored', 'user', 'first'), + msg('2-assistant-stored', 'assistant', 'first answer'), + msg('3-marker-stored', 'user', '[System: The active model for this chat has changed to k3.]') + ] + + expect(preserveLocalPendingTurnMessages(next, previous).map(message => message.id)).toEqual([ + '1-user-stored', + '2-assistant-stored', + '3-marker-stored', + 'user-optimistic' + ]) + }) }) describe('appendLiveSessionProjection', () => { diff --git a/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts b/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts index f724a8f3e2cf..55781c3bcb7a 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts @@ -240,7 +240,17 @@ export function reconcileResumeMessages(nextMessages: ChatMessage[], previousMes * history, not in-flight work. The latest authoritative user confirms whether * that tail has persisted; any authoritative assistant at the same ordinal * supersedes the local stream. + * + * Gateway bookkeeping markers (the model-switch / personality notices written + * by tui_gateway/server.py) are persisted as role=user but are not user turns. + * They must not take part in ordinal pairing on either side: a stored marker + * between two real user turns shifts every later user ordinal, so the optimistic + * row misses its committed copy and is appended a second time at the end of the + * transcript — the duplicated user bubble of #67603. */ +const isGatewaySystemMarker = (message: ChatMessage): boolean => + message.role === 'user' && chatMessageText(message).trimStart().startsWith('[System:') + export function preserveLocalPendingTurnMessages( nextMessages: ChatMessage[], previousMessages: ChatMessage[] @@ -253,6 +263,10 @@ export function preserveLocalPendingTurnMessages( const nextRoleCounts = new Map() for (const message of nextMessages) { + if (isGatewaySystemMarker(message)) { + continue + } + const ordinal = nextRoleCounts.get(message.role) ?? 0 nextRoleCounts.set(message.role, ordinal + 1) nextByRoleOrdinal.set(`${message.role}:${ordinal}`, message) @@ -269,6 +283,10 @@ export function preserveLocalPendingTurnMessages( const preserved: ChatMessage[] = [] for (const message of previousMessages) { + if (isGatewaySystemMarker(message)) { + continue + } + const ordinal = previousRoleCounts.get(message.role) ?? 0 previousRoleCounts.set(message.role, ordinal + 1) @@ -497,6 +515,28 @@ export async function resolveStoredSession(storedSessionId: string): Promise { + if (!storedSessionId) { + return undefined + } + + const profile = (await resolveStoredSession(storedSessionId))?.profile?.trim() + + return profile || undefined +} + type SessionRuntimeStatePatch = Partial< Pick< ClientSessionState,