diff --git a/apps/desktop/src/app/contrib/hooks/live-status-reap.test.ts b/apps/desktop/src/app/contrib/hooks/live-status-reap.test.ts new file mode 100644 index 000000000000..8b4688331c0e --- /dev/null +++ b/apps/desktop/src/app/contrib/hooks/live-status-reap.test.ts @@ -0,0 +1,79 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { $selectedStoredSessionId, $unreadFinishedSessionIds } from '@/store/session' +import { $attentionSessionIds, $workingSessionIds, clearAllSessionStates } from '@/store/session-states' + +import { rehydrateLiveSessionStatuses } from './use-background-sync' + +/** + * `session.active_list` is the authoritative snapshot of what is RUNNING in the + * polled gateway process. A session that finished while Desktop was looking + * elsewhere — or whose runtime id was recycled by a backend respawn — simply + * stops appearing in the response. Absence is therefore a completion signal, + * not "no news": if nothing reaps it, the row spins forever and the + * busy→idle edge that paints the green "your turn" dot never fires. + */ +describe('rehydrateLiveSessionStatuses — reaping vanished runtimes', () => { + beforeEach(() => { + vi.useFakeTimers() + $selectedStoredSessionId.set(null) + $unreadFinishedSessionIds.set([]) + }) + + afterEach(() => { + vi.clearAllTimers() + vi.useRealTimers() + clearAllSessionStates() + $unreadFinishedSessionIds.set([]) + }) + + it('clears a working session that disappears from the live snapshot', () => { + rehydrateLiveSessionStatuses({ + sessions: [{ id: 'runtime-a', session_key: 'stored-a', status: 'working' }] + }) + + expect($workingSessionIds.get()).toEqual(['stored-a']) + + // The turn finished and the gateway reaped the session between polls. + rehydrateLiveSessionStatuses({ sessions: [] }) + + expect($workingSessionIds.get()).toEqual([]) + }) + + it('fires the unread "your turn" marker for a vanished background session', () => { + rehydrateLiveSessionStatuses({ + sessions: [{ id: 'runtime-b', session_key: 'stored-b', status: 'working' }] + }) + + rehydrateLiveSessionStatuses({ sessions: [] }) + + expect($unreadFinishedSessionIds.get()).toEqual(['stored-b']) + }) + + it('clears a blocked session that disappears from the live snapshot', () => { + rehydrateLiveSessionStatuses({ + sessions: [{ id: 'runtime-c', session_key: 'stored-c', status: 'waiting' }] + }) + + expect($attentionSessionIds.get()).toEqual(['stored-c']) + + rehydrateLiveSessionStatuses({ sessions: [] }) + + expect($attentionSessionIds.get()).toEqual([]) + }) + + it('leaves runtimes this poll never seeded alone', () => { + // A background PROFILE's sessions are served by a different gateway and + // never appear in this profile's active_list. Reaping them would dark out + // every other profile's running rows. + rehydrateLiveSessionStatuses( + { sessions: [{ id: 'runtime-other', session_key: 'stored-other', status: 'working' }] }, + Date.now(), + 'other' + ) + + rehydrateLiveSessionStatuses({ sessions: [] }, Date.now(), 'default') + + expect($workingSessionIds.get()).toEqual(['stored-other']) + }) +}) diff --git a/apps/desktop/src/app/contrib/hooks/live-status-spinner.test.ts b/apps/desktop/src/app/contrib/hooks/live-status-spinner.test.ts new file mode 100644 index 000000000000..4fcad3b1a2ed --- /dev/null +++ b/apps/desktop/src/app/contrib/hooks/live-status-spinner.test.ts @@ -0,0 +1,77 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { $selectedStoredSessionId, $unreadFinishedSessionIds } from '@/store/session' +import { $workingSessionIds, clearAllSessionStates } from '@/store/session-states' + +import { rehydrateLiveSessionStatuses, resetLiveRuntimeTracking } from './use-background-sync' + +/** + * (C) The sidebar spinner is driven by `$workingSessionIds`, which is keyed by + * STORED session id. A turn that STARTS while Desktop isn't receiving stream + * events — a background profile, a degraded remote socket, a session opened on + * another surface — is only ever learned about through the `session.active_list` + * poll. If that poll can't seed a row the renderer has never seen, the thread + * name never gets its arc even though the backend is plainly working. + */ +describe('rehydrateLiveSessionStatuses — seeding a turn the renderer never saw start', () => { + beforeEach(() => { + vi.useFakeTimers() + $selectedStoredSessionId.set(null) + $unreadFinishedSessionIds.set([]) + resetLiveRuntimeTracking() + }) + + afterEach(() => { + vi.clearAllTimers() + vi.useRealTimers() + clearAllSessionStates() + resetLiveRuntimeTracking() + $unreadFinishedSessionIds.set([]) + }) + + it('shows the spinner for a turn that started with no stream events', () => { + rehydrateLiveSessionStatuses({ + sessions: [{ id: 'runtime-cold', session_key: 'stored-cold', status: 'working' }] + }) + + expect($workingSessionIds.get()).toContain('stored-cold') + }) + + it('keeps the spinner across polls while the turn is still running', () => { + rehydrateLiveSessionStatuses({ + sessions: [{ id: 'runtime-cold', session_key: 'stored-cold', status: 'working' }] + }) + rehydrateLiveSessionStatuses({ + sessions: [{ id: 'runtime-cold', session_key: 'stored-cold', status: 'working' }] + }) + + expect($workingSessionIds.get()).toContain('stored-cold') + }) + + it('shows the spinner when a runtime id is recycled onto a new stored session', () => { + // A respawned backend can mint the same runtime id for a different stored + // session. The row for the NEW stored id must light up, not the stale one. + rehydrateLiveSessionStatuses({ + sessions: [{ id: 'runtime-1', session_key: 'stored-old', status: 'working' }] + }) + + rehydrateLiveSessionStatuses({ + sessions: [{ id: 'runtime-1', session_key: 'stored-new', status: 'working' }] + }) + + expect($workingSessionIds.get()).toContain('stored-new') + expect($workingSessionIds.get()).not.toContain('stored-old') + }) + + it('leaves a starting session idle — the agent build is not proof of a turn', () => { + // `starting` = `agent_build_started` without `agent_ready`. _start_agent_build + // runs on the first prompt OR any incidental RPC that needs the agent, so it + // is not proof of a turn — lighting the spinner here would fire on merely + // opening a session. A real turn arrives as `working`. + rehydrateLiveSessionStatuses({ + sessions: [{ id: 'runtime-boot', session_key: 'stored-boot', status: 'starting' }] + }) + + expect($workingSessionIds.get()).not.toContain('stored-boot') + }) +}) diff --git a/apps/desktop/src/app/contrib/hooks/use-background-sync.ts b/apps/desktop/src/app/contrib/hooks/use-background-sync.ts index 5925b4a7dfbe..68a802dbc91d 100644 --- a/apps/desktop/src/app/contrib/hooks/use-background-sync.ts +++ b/apps/desktop/src/app/contrib/hooks/use-background-sync.ts @@ -37,11 +37,30 @@ interface LiveSessionStatusResponse { sessions?: LiveSessionStatusItem[] } +// Runtime ids this poll has seen live, per gateway profile. A profile only +// ever reaps what its OWN snapshot previously reported: background profiles are +// served by different gateways and never appear in this profile's active_list, +// so an unscoped reap would dark out every other profile's running rows. +const liveRuntimeIdsByProfile = new Map>() + /** Restore sidebar liveness after a renderer/backend reconnect. Stream events * normally own these states, but events emitted while Desktop was disconnected * cannot be replayed. `session.active_list` is the authoritative in-memory - * snapshot and does not resume, focus, or otherwise mutate a chat. */ -export function rehydrateLiveSessionStatuses(response: LiveSessionStatusResponse, nowMs = Date.now()): void { + * snapshot and does not resume, focus, or otherwise mutate a chat. + * + * The snapshot is authoritative about ABSENCE too. A turn that ends while the + * websocket is degraded — a remote gateway over a flaky link, a reconnect, a + * profile swap — drops out of `_sessions` without Desktop ever seeing the + * `running: false` edge, so the row keeps spinning and the busy→idle transition + * that paints the green "your turn" dot never fires. Reaping runtimes that + * vanish between polls restores both. */ +export function rehydrateLiveSessionStatuses( + response: LiveSessionStatusResponse, + nowMs = Date.now(), + profileKey = 'default' +): void { + const seen = new Set() + for (const session of response.sessions ?? []) { const runtimeSessionId = session.id?.trim() const storedSessionId = session.session_key?.trim() @@ -52,6 +71,8 @@ export function rehydrateLiveSessionStatuses(response: LiveSessionStatusResponse continue } + seen.add(runtimeSessionId) + const existing = $sessionStates.get()[runtimeSessionId] // Avoid re-arming the watchdog on every poll. Publish only when the @@ -87,6 +108,44 @@ export function rehydrateLiveSessionStatuses(response: LiveSessionStatusResponse setSessionStalled(storedSessionId, isQuiet) } + + // A runtime this profile's snapshot reported live LAST poll but not this one + // has ended: the gateway reaps a session out of `_sessions` when its turn + // completes and its transport goes away. Settle it through the normal publish + // path so the busy→idle transition fires — that edge is what clears the + // spinner AND marks the row unread ("your turn"). Only ids this profile + // previously saw are eligible, so another profile's live rows are untouched. + const previouslyLive = liveRuntimeIdsByProfile.get(profileKey) + + if (previouslyLive) { + for (const runtimeSessionId of previouslyLive) { + if (seen.has(runtimeSessionId)) { + continue + } + + const existing = $sessionStates.get()[runtimeSessionId] + + if (existing?.busy || existing?.needsInput) { + publishSessionState(runtimeSessionId, { + ...existing, + awaitingResponse: false, + busy: false, + needsInput: false, + streamId: null, + turnStartedAt: null + }) + } + } + } + + liveRuntimeIdsByProfile.set(profileKey, seen) +} + +/** Forget every profile's live-runtime bookkeeping. A gateway wipe already + * drops the session states these ids point at, so a carried-over set would + * only reap runtimes that no longer exist. */ +export function resetLiveRuntimeTracking(): void { + liveRuntimeIdsByProfile.clear() } interface BackgroundSyncParams { @@ -191,7 +250,7 @@ export function useBackgroundSync({ const response = await requestGateway('session.active_list', {}) if (!cancelled) { - rehydrateLiveSessionStatuses(response) + rehydrateLiveSessionStatuses(response, Date.now(), activeGatewayProfile) } } catch { // Older gateways may not expose session.active_list. Live stream events diff --git a/apps/desktop/src/app/session/hooks/use-session-actions/resume-structural-parts.test.ts b/apps/desktop/src/app/session/hooks/use-session-actions/resume-structural-parts.test.ts new file mode 100644 index 000000000000..1ef5376d8d96 --- /dev/null +++ b/apps/desktop/src/app/session/hooks/use-session-actions/resume-structural-parts.test.ts @@ -0,0 +1,76 @@ +import { describe, expect, it } from 'vitest' + +import type { ChatMessage } from '@/lib/chat-messages' + +import { reconcileResumeMessages } from './utils' + +const user = (id: string, text: string): ChatMessage => ({ + id, + parts: [{ type: 'text', text }], + role: 'user' +}) + +/** + * Switching away from a running session and back re-hydrates it from + * `session.activate`, whose transcript is TEXT-ONLY for the live turn (the + * gateway's `inflight` projection carries `user`/`assistant` strings, nothing + * structural). The renderer's cached state is the only carrier of the turn's + * tool calls and reasoning, so reconcile must not drop them. + */ +describe('reconcileResumeMessages — structural parts on a mid-turn switch', () => { + it('keeps tool-call parts the authoritative text-only row cannot carry', () => { + const cached: ChatMessage[] = [ + user('u1', 'read the config'), + { + id: 'a1', + parts: [ + { type: 'reasoning', text: 'I should read the file first.' }, + { type: 'tool-call', toolCallId: 'call-1', toolName: 'read_file', result: 'contents' }, + { type: 'text', text: 'Reading it now' } + ], + role: 'assistant' + } + ] + + // What activate returns mid-turn: the same rows, but flattened to text and + // one delta further along, so the text no longer matches the cached copy. + const authoritative: ChatMessage[] = [ + user('u1', 'read the config'), + { id: 'a1', parts: [{ type: 'text', text: 'Reading it now — found the key' }], role: 'assistant' } + ] + + const [, assistant] = reconcileResumeMessages(authoritative, cached) + + expect(assistant.parts.filter(p => p.type === 'tool-call')).toHaveLength(1) + expect(assistant.parts.filter(p => p.type === 'reasoning')).toHaveLength(1) + // The newer authoritative text still wins. + expect(assistant.parts.filter(p => p.type === 'text').at(-1)).toMatchObject({ + text: 'Reading it now — found the key' + }) + }) + + it('does not duplicate tool calls the authoritative row already carries', () => { + const cached: ChatMessage[] = [ + { + id: 'a1', + parts: [{ type: 'tool-call', toolCallId: 'call-1', toolName: 'read_file', result: 'contents' }], + role: 'assistant' + } + ] + + const authoritative: ChatMessage[] = [ + { + id: 'a1', + parts: [ + { type: 'tool-call', toolCallId: 'call-1', toolName: 'read_file', result: 'contents' }, + { type: 'text', text: 'done' } + ], + role: 'assistant' + } + ] + + const [assistant] = reconcileResumeMessages(authoritative, cached) + + expect(assistant.parts.filter(p => p.type === 'tool-call')).toHaveLength(1) + }) +}) 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 39ba0bd911c6..f06069ca75fd 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 @@ -46,14 +46,40 @@ function withAppendedText(message: ChatMessage, suffix: string): ChatMessage { return appended ? { ...message, parts } : message } -function preserveReasoningParts(message: ChatMessage, previous: ChatMessage): ChatMessage { - if (message.parts.some(part => part.type === 'reasoning')) { +/** + * Carry structural parts an authoritative row cannot express. + * + * A live turn's authoritative projection is TEXT-ONLY: the gateway's `inflight` + * snapshot carries `user`/`assistant` strings, and history is not committed + * until the turn finishes. The renderer's cached state is therefore the sole + * carrier of the running turn's reasoning and tool calls, so switching threads + * mid-turn and back re-hydrated an assistant row stripped of both — the turn + * looked inert, with no thinking trace and no tool activity. + * + * Preserved only when the rows are the SAME turn: identical text, or the + * authoritative text extending the cached one (another delta landed). Anything + * else may be a different turn at the same role ordinal — compression rewrites + * history — and must not inherit foreign parts. Tool calls dedupe on + * `toolCallId` so a row that already carries them is left alone. + */ +function preserveStructuralParts(message: ChatMessage, previous: ChatMessage): ChatMessage { + const carried = previous.parts.filter(part => part.type === 'reasoning' || part.type === 'tool-call') + + if (!carried.length) { return message } - const reasoningParts = previous.parts.filter(part => part.type === 'reasoning') + const hasReasoning = message.parts.some(part => part.type === 'reasoning') - return reasoningParts.length ? { ...message, parts: [...reasoningParts, ...message.parts] } : message + const presentToolCallIds = new Set( + message.parts.flatMap(part => (part.type === 'tool-call' ? [part.toolCallId] : [])) + ) + + const missing = carried.filter(part => + part.type === 'reasoning' ? !hasReasoning : !presentToolCallIds.has(part.toolCallId) + ) + + return missing.length ? { ...message, parts: [...missing, ...message.parts] } : message } // Compile-time exhaustiveness guards. If a new field is added to ChatMessage @@ -209,12 +235,29 @@ export function reconcileResumeMessages(nextMessages: ChatMessage[], previousMes const previousVisibleText = textWithoutEmbeddedImages(previousText) let preserved = message - if (nextText === previousVisibleText || nextText === previousText.trim()) { - preserved = preserveReasoningParts(preserved, previous) + const sameText = nextText === previousVisibleText || nextText === previousText.trim() - if (message.role === 'user' && preserved.attachmentRefs === undefined && previous.attachmentRefs?.length) { - preserved = { ...preserved, attachmentRefs: [...previous.attachmentRefs] } - } + // Mid-turn, the authoritative text has advanced past the cached copy by one + // or more deltas. That is still the same turn, and the cached row holds the + // only copy of its reasoning / tool calls, so treat an extension as a match + // for structural carry-over. Attachment refs and image re-appending stay on + // the strict equality path — they reconcile a SETTLED row, and a growing + // row is by definition not settled. + const sameTurn = + sameText || + (nextText.length > 0 && previousVisibleText.length > 0 && nextText.startsWith(previousVisibleText.trim())) + + if (sameTurn) { + preserved = preserveStructuralParts(preserved, previous) + } + + if ( + sameText && + message.role === 'user' && + preserved.attachmentRefs === undefined && + previous.attachmentRefs?.length + ) { + preserved = { ...preserved, attachmentRefs: [...previous.attachmentRefs] } } const previousImages = embeddedImageUrls(previousText) diff --git a/apps/desktop/src/store/gateway-switch.ts b/apps/desktop/src/store/gateway-switch.ts index 291b803283a9..df5227af9e10 100644 --- a/apps/desktop/src/store/gateway-switch.ts +++ b/apps/desktop/src/store/gateway-switch.ts @@ -1,5 +1,6 @@ import { atom } from 'nanostores' +import { resetLiveRuntimeTracking } from '@/app/contrib/hooks/use-background-sync' import { resetSidebarBatchCapability } from '@/hermes' import { invalidateProfileScopedQueries } from '@/lib/query-client' import { resetSessionsLimit } from '@/store/layout' @@ -52,6 +53,7 @@ export function wipeSessionListsForGatewaySwitch(): void { // $attentionSessionIds (computed) and $stalledSessionIds (owned beside it). // $unreadFinishedSessionIds is separate, so wipe it explicitly. clearAllSessionStates() + resetLiveRuntimeTracking() $unreadFinishedSessionIds.set([]) setSessionsLoading(true) resetSessionsLimit() diff --git a/apps/desktop/src/store/working-ids-stored-id.test.ts b/apps/desktop/src/store/working-ids-stored-id.test.ts new file mode 100644 index 000000000000..f302385a4442 --- /dev/null +++ b/apps/desktop/src/store/working-ids-stored-id.test.ts @@ -0,0 +1,32 @@ +import { afterEach, describe, expect, it } from 'vitest' + +import { createClientSessionState } from '@/lib/chat-runtime' +import { $workingSessionIds, clearAllSessionStates, publishSessionState } from '@/store/session-states' + +/** + * (C) The sidebar spinner reads `$workingSessionIds`, which projects + * `$sessionStates` down to STORED session ids and drops any entry whose + * `storedSessionId` is null. `message.start` flips `busy` without carrying a + * stored id, so a runtime that was never seeded with one goes busy invisibly: + * the backend works, the thread name stays bare. + */ +describe('$workingSessionIds — a busy runtime with no stored id', () => { + afterEach(() => { + clearAllSessionStates() + }) + + it('cannot show a spinner for a busy runtime that has no stored id', () => { + publishSessionState('runtime-unmapped', { ...createClientSessionState(null), busy: true }) + + // Documents the constraint rather than asserting the bug is fine: the + // projection is keyed by stored id, so an unmapped runtime is unreachable + // from the sidebar no matter how busy it is. + expect($workingSessionIds.get()).toEqual([]) + }) + + it('shows the spinner as soon as the stored id is known', () => { + publishSessionState('runtime-mapped', { ...createClientSessionState('stored-x'), busy: true }) + + expect($workingSessionIds.get()).toEqual(['stored-x']) + }) +})