Merge pull request #72303 from NousResearch/bb/desktop-session-status

fix(desktop): stop sidebar sessions from lying about whether they're running

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'])
+  })
+})

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')
+  })
+})

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<string, Set<string>>()
+
 /** 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<string>()
+
   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<LiveSessionStatusResponse>('session.active_list', {})
 
         if (!cancelled) {
-          rehydrateLiveSessionStatuses(response)
+          rehydrateLiveSessionStatuses(response, Date.now(), activeGatewayProfile)
         }
       } catch {
         // Older gateways may not expose session.active_list. Live stream events

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)
+  })
+})

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)

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()

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'])
+  })
+})