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
27 changes: 22 additions & 5 deletions apps/desktop/src/app/contrib/hooks/use-desktop-integrations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,15 @@ import { useEffect, useRef } from 'react'
import { closeActiveTab } from '@/app/chat/close-tab'
import { storedSessionIdForNotification } from '@/lib/session-ids'
import { respondToApprovalAction } from '@/store/native-notifications'
import { getRememberedRoute, getRememberedSessionId, setRememberedRoute, setRememberedSessionId } from '@/store/session'
import { $activeGatewayProfile } from '@/store/profile'
import {
$sessions,
getRememberedRoute,
getRememberedSessionId,
rememberedSessionProfile,
setRememberedRoute,
setRememberedSessionId
} from '@/store/session'
import { onSessionsChanged } from '@/store/session-sync'
import { openUpdatesWindow, startUpdatePoller, stopUpdatePoller } from '@/store/updates'
import { isSecondaryWindow } from '@/store/windows'
Expand Down Expand Up @@ -63,7 +71,10 @@ export function useDesktopIntegrations({
// you don't want to boot into a modal.
useEffect(() => {
if (routedSessionId) {
setRememberedSessionId(routedSessionId)
setRememberedSessionId(
routedSessionId,
rememberedSessionProfile($sessions.get(), routedSessionId, $activeGatewayProfile.get())
)
}

if (!isOverlayView(appViewForPath(locationPathname))) {
Expand Down Expand Up @@ -92,16 +103,22 @@ export function useDesktopIntegrations({
return
}

const last = getRememberedSessionId()
const last = getRememberedSessionId($activeGatewayProfile.get())

if (last) {
navigate(sessionRoute(last), { replace: true })
}
}, [locationPathname, navigate])

useEffect(() => {
if (resumeExhaustedSessionId && getRememberedSessionId() === resumeExhaustedSessionId) {
setRememberedSessionId(null)
if (!resumeExhaustedSessionId) {
return
}

const owner = rememberedSessionProfile($sessions.get(), resumeExhaustedSessionId, $activeGatewayProfile.get())

if (getRememberedSessionId(owner) === resumeExhaustedSessionId) {
setRememberedSessionId(null, owner)
}
}, [resumeExhaustedSessionId])

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
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'
Expand All @@ -25,6 +26,7 @@

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()
Expand Down Expand Up @@ -182,7 +184,7 @@
submitText: (...args: Parameters<typeof actions.submitText>) =>
act(async () => actions.submitText(...args)) as Promise<boolean>
})
}, [

Check warning on line 187 in apps/desktop/src/app/session/hooks/use-prompt-actions/index.test.tsx

View workflow job for this annotation

GitHub Actions / JS & TS checks / Typecheck & Test (apps/desktop)

React Hook useEffect has a missing dependency: 'actions'. Either include it or remove the dependency array
actions.cancelRun,
actions.restoreToMessage,
actions.redirectPrompt,
Expand Down Expand Up @@ -1834,6 +1836,99 @@
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<string, unknown> }[] = []
let submitAttempts = 0

const requestGateway = vi.fn(async (method: string, params?: Record<string, unknown>) => {
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(
<Harness
onReady={h => (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<string, unknown> }[] = []
let submitAttempts = 0

const requestGateway = vi.fn(async (method: string, params?: Record<string, unknown>) => {
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(
<Harness
onReady={h => (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<string, unknown> }[] = []
let submitAttempts = 0
Expand Down
11 changes: 9 additions & 2 deletions apps/desktop/src/app/session/hooks/use-prompt-actions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import type {
ImageAttachResponse,
SessionRedirectResponse
} from '../../../types'
import { resolveSessionProfile } from '../use-session-actions/utils'

import {
applyBranchVisibility,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
13 changes: 11 additions & 2 deletions apps/desktop/src/app/session/hooks/use-prompt-actions/submit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down
36 changes: 35 additions & 1 deletion apps/desktop/src/app/session/hooks/use-session-actions.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -43,6 +43,7 @@ import { useSessionActions } from './use-session-actions'
vi.mock('@/hermes', async importOriginal => ({
...(await importOriginal<Record<string, unknown>>()),
deleteSession: vi.fn(),
getSession: vi.fn(),
getSessionMessages: vi.fn(),
listAllProfileSessions: vi.fn(),
setApiRequestProfile: vi.fn(),
Expand Down Expand Up @@ -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<boolean>) | null =
null
render(<BranchHarness onReady={branch => (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) ─────────
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1177,7 +1177,13 @@ export function useSessionActions({
async (storedSessionId: string, sessionProfile?: string | null): Promise<boolean> => {
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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Loading
Loading