From 42a30d13dbcf572585cc566f37d0c28d69f4c3d1 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Sun, 26 Jul 2026 18:39:03 -0500 Subject: [PATCH 1/3] fix(desktop): settle sessions that vanish from the live snapshot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit session.active_list is authoritative about absence, but the renderer only read the rows it returned. A turn that ends while the websocket is degraded — a remote gateway on a flaky link, a reconnect, a profile swap — drops out of the gateway's _sessions without Desktop ever seeing the running=false edge, so the row spins forever and the busy->idle transition that paints the green unread dot never fires. Track live runtime ids per gateway profile and settle anything that disappears between polls through publishSessionState so the real transition fires. Profile scoping is load-bearing: background profiles are served by other gateways and never appear in this profile's snapshot, so an unscoped reap would dark out every other profile's running rows. --- .../contrib/hooks/live-status-reap.test.ts | 79 +++++++++++++++++++ .../app/contrib/hooks/use-background-sync.ts | 65 ++++++++++++++- apps/desktop/src/store/gateway-switch.ts | 2 + 3 files changed, 143 insertions(+), 3 deletions(-) create mode 100644 apps/desktop/src/app/contrib/hooks/live-status-reap.test.ts 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/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/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() From 6b273f419a2ebf3e67fbe2a580def311c29224f6 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Sun, 26 Jul 2026 18:39:11 -0500 Subject: [PATCH 2/3] fix(desktop): keep thinking traces and tool calls across a mid-turn switch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit preserveReasoningParts was gated on exact text equality with the cached row. Mid-turn the authoritative text has advanced by a delta or two, so the guard fails and the row is rebuilt from the gateway's inflight projection — which is text-only. The renderer's cache is the sole carrier of a running turn's structure, so switching away and back stripped the reasoning and every tool call, leaving the turn looking inert. Carry tool calls alongside reasoning, dedupe them on toolCallId, and match on same-turn (identical text, or authoritative text extending the cached text) rather than strict equality. Attachment refs and image re-appending stay on the strict path: those reconcile a settled row, and a growing row is by definition not settled. --- .../resume-structural-parts.test.ts | 76 +++++++++++++++++++ .../hooks/use-session-actions/utils.ts | 61 ++++++++++++--- 2 files changed, 128 insertions(+), 9 deletions(-) create mode 100644 apps/desktop/src/app/session/hooks/use-session-actions/resume-structural-parts.test.ts 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) From 19d84d10715bf43986cdab9afe445d2a79c1bf04 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Sun, 26 Jul 2026 19:02:46 -0500 Subject: [PATCH 3/3] test(desktop): pin the sidebar spinner's liveness contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Investigating the missing-spinner report turned up no defect in the seeding path: the active_list poll already lights a row for a turn the renderer never saw start, holds it across polls, and follows a recycled runtime id onto its new stored session. Pin all three so the reap change can't silently regress turn-start while fixing turn-end. Two boundaries worth naming rather than rediscovering: - `starting` is deliberately NOT working. It means agent_build_started without agent_ready, and _start_agent_build runs on any incidental RPC that needs the agent — not just a prompt — so treating it as a turn would spin the row on merely opening a session. - $workingSessionIds is keyed by STORED id and drops entries whose storedSessionId is null, while message.start flips busy without carrying one. A runtime that was never seeded with a stored id therefore goes busy invisibly. That is the remaining path by which a spinner can go missing. --- .../contrib/hooks/live-status-spinner.test.ts | 77 +++++++++++++++++++ .../src/store/working-ids-stored-id.test.ts | 32 ++++++++ 2 files changed, 109 insertions(+) create mode 100644 apps/desktop/src/app/contrib/hooks/live-status-spinner.test.ts create mode 100644 apps/desktop/src/store/working-ids-stored-id.test.ts 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/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']) + }) +})