fix(desktop): keep completed subagents from reviving stale activity

2026-09-15 23:41:02 -07:00
parent f0b2ba4160
commit 57d34b14c2
6 changed files with 154 additions and 124 deletions
@@ -0,0 +1,42 @@
+import { act, cleanup, render } from '@testing-library/react'
+import { atom } from 'nanostores'
+import { afterEach, expect, it } from 'vitest'
+
+import { PRIMARY_SESSION_VIEW, SessionViewProvider } from '@/app/chat/session-view'
+import { $activeSessionId, $busy } from '@/store/session'
+import { $subagentsBySession, upsertSubagent } from '@/store/subagents'
+
+import { BackgroundResumeNotice } from './status'
+
+afterEach(() => {
+  cleanup()
+  $activeSessionId.set(null)
+  $busy.set(false)
+  $subagentsBySession.set({})
+})
+
+it('keeps background waiting local to its idle thread without echoing child thinking chrome', () => {
+  $activeSessionId.set('other')
+  $busy.set(true)
+  upsertSubagent('owner', { subagent_id: 'child', goal: 'Check relevance', status: 'running' })
+  upsertSubagent('owner', { subagent_id: 'child', text: '(°□°) pondering...' }, false, 'subagent.thinking')
+  const $ownerBusy = atom(false)
+  const view = { ...PRIMARY_SESSION_VIEW, $runtimeId: atom<string | null>('owner'), $busy: $ownerBusy }
+
+  const { container } = render(
+    <SessionViewProvider value={view}>
+      <BackgroundResumeNotice />
+    </SessionViewProvider>
+  )
+
+  expect(container.querySelector('[role="status"]')).toBeTruthy()
+  expect(container.textContent).not.toContain('pondering')
+  expect(container.querySelector('.shimmer')).toBeNull()
+
+  act(() => $ownerBusy.set(true))
+  expect(container.querySelector('[role="status"]')).toBeNull()
+  act(() => $ownerBusy.set(false))
+  expect(container.querySelector('[role="status"]')).toBeTruthy()
+  act(() => upsertSubagent('owner', { subagent_id: 'child', status: 'completed' }, true, 'subagent.complete'))
+  expect(container.querySelector('[role="status"]')).toBeNull()
+})
@@ -14,7 +14,7 @@ import { StatusPulse } from '@/components/ui/status-pulse'
 import { getLocalModelsStatus } from '@/hermes'
 import { useI18n } from '@/i18n'
 import { cn } from '@/lib/utils'
-import { $backgroundResume } from '@/store/background-delegation'
+import { sessionBackgroundResume } from '@/store/background-delegation'
 import { sessionCompacting } from '@/store/compaction'
 import { $localModelsEnabled } from '@/store/local-models-flag'
 import { sessionAwaitingInput } from '@/store/prompts'
@@ -267,33 +267,26 @@ export const ResponseLoadingIndicator: FC = () => {
  )
 }

-// Parked-background affordance: a top-level delegate_task runs in the
-// background, so the parent turn ends and the app goes idle while the subagent
-// keeps working and its result re-enters as a fresh turn later. Instead of a
-// spinner (reads as "stuck"), reuse the same compact, centered system-note
-// chrome as the steer / slash-status lines (SystemMessage above) so it sits in
-// the thread like every other meta line. Idle-only (gated upstream). Null when
-// nothing is parked.
+// The parent is idle while its delegated children work. Name that wait rather
+// than echoing the child's CLI thinking spinner as if this thread were running.
 export const BackgroundResumeNotice: FC = () => {
  const { t } = useI18n()
-  const resume = useStore($backgroundResume)
+  const view = useSessionView()
+  const sessionId = useStore(view.$runtimeId)
+  const busy = useStore(view.$busy)
+  const resume = useStore(useMemo(() => sessionBackgroundResume(sessionId), [sessionId]))

-  if (!resume) {
+  if (busy || !resume) {
    return null
  }

-  const label = resume.activity ?? t.assistant.thread.resumeWhenBackgroundDone(resume.count)
+  const label = t.assistant.thread.resumeWhenBackgroundDone(resume.count)

  return (
-    <div
-      aria-live="polite"
-      className="flex max-w-[min(86%,44rem)] items-center gap-1.5 self-center px-2 py-0.5 text-[0.6875rem] leading-5 text-muted-foreground/55"
-      data-slot="aui_background-resume"
-      role="status"
-    >
-      <Codicon className="text-muted-foreground/55" name="sync" size="0.75rem" />
-      <span className="shimmer min-w-0 truncate">{label}</span>
-    </div>
+    <StatusRow className="pl-(--message-text-indent)" data-slot="aui_background-resume" label={label}>
+      <Codicon name="sync" size="0.875rem" />
+      <span className={cn(SCAFFOLD_LABEL_CLASS, 'min-w-0 truncate')}>{label}</span>
+    </StatusRow>
  )
 }

@@ -1,65 +1,34 @@
-import { beforeEach, describe, expect, it } from 'vitest'
+import { afterEach, expect, it } from 'vitest'

-import { $backgroundResume } from './background-delegation'
+import { sessionBackgroundResume } from './background-delegation'
 import { $activeSessionId, $busy } from './session'
-import { $subagentsBySession, type SubagentProgress, type SubagentStreamEntry } from './subagents'
+import { $subagentsBySession, upsertSubagent } from './subagents'

-const sub = (over: Partial<SubagentProgress> = {}): SubagentProgress => ({
-  id: over.id ?? 'deleg:1',
-  parentId: null,
-  goal: 'do the thing',
-  status: 'running',
-  taskCount: 1,
-  taskIndex: 0,
-  startedAt: 0,
-  updatedAt: 0,
-  filesRead: [],
-  filesWritten: [],
-  stream: [],
-  ...over
+afterEach(() => {
+  $busy.set(false)
+  $activeSessionId.set(null)
+  $subagentsBySession.set({})
 })

-const stream = (text: string): SubagentStreamEntry => ({ at: 0, kind: 'progress', text })
+it('tracks only the owning runtime session, independent of foreground busy state and child stream text', () => {
+  const owner = sessionBackgroundResume('owner')
+  const other = sessionBackgroundResume('other')
+  const unbound = sessionBackgroundResume(null)
+  upsertSubagent('owner', { subagent_id: 'working', status: 'running' }, true, 'subagent.start')
+  upsertSubagent('owner', { subagent_id: 'waiting', status: 'queued' }, true, 'subagent.spawn_requested')
+  upsertSubagent('other', { subagent_id: 'working', status: 'running' }, true, 'subagent.start')
+  $activeSessionId.set('other')
+  $busy.set(true)
+  upsertSubagent('owner', { subagent_id: 'working', text: '(°□°) pondering...' }, false, 'subagent.thinking')
+  expect(owner.get()).toEqual({ count: 2 })
+  expect(other.get()).toEqual({ count: 1 })
+  expect(unbound.get()).toBeNull()

-describe('$backgroundResume', () => {
-  beforeEach(() => {
-    $busy.set(false)
-    $activeSessionId.set('s1')
-    $subagentsBySession.set({})
-  })
-
-  it('counts running/queued children for the active session while idle', () => {
-    $subagentsBySession.set({ s1: [sub({ id: 'a' }), sub({ id: 'b', status: 'queued' })] })
-    expect($backgroundResume.get()?.count).toBe(2)
-  })
-
-  it('surfaces the primary child latest stream line as live activity', () => {
-    $subagentsBySession.set({ s1: [sub({ id: 'a', stream: [stream('Searching the web…')] })] })
-    expect($backgroundResume.get()?.activity).toBe('Searching the web…')
-  })
-
-  it('activity is null when no stream line has arrived (UI uses generic copy)', () => {
-    $subagentsBySession.set({ s1: [sub({ id: 'a' })] })
-    expect($backgroundResume.get()?.activity).toBeNull()
-  })
-
-  it('is null while a turn is busy (the turn owns the main loader)', () => {
-    $subagentsBySession.set({ s1: [sub({ id: 'a' })] })
-    $busy.set(true)
-    expect($backgroundResume.get()).toBeNull()
-  })
-
-  it('is null when only terminal children or other sessions have work', () => {
-    $subagentsBySession.set({
-      s1: [sub({ id: 'a', status: 'completed' }), sub({ id: 'b', status: 'failed' })],
-      s2: [sub({ id: 'c' })]
-    })
-    expect($backgroundResume.get()).toBeNull()
-  })
-
-  it('is null when there is no active session', () => {
-    $subagentsBySession.set({ s1: [sub({ id: 'a' })] })
-    $activeSessionId.set(null)
-    expect($backgroundResume.get()).toBeNull()
-  })
+  upsertSubagent('owner', { subagent_id: 'working', status: 'completed' }, false, 'subagent.complete')
+  expect(owner.get()).toEqual({ count: 1 })
+  upsertSubagent('owner', { subagent_id: 'waiting', status: 'timeout' }, false, 'subagent.complete')
+  $activeSessionId.set(null)
+  $busy.set(false)
+  expect(owner.get()).toBeNull()
+  expect(other.get()).toEqual({ count: 1 })
 })
@@ -1,48 +1,19 @@
 import { computed } from 'nanostores'

-import { $activeSessionId, $busy } from './session'
-import { $subagentsBySession, type SubagentProgress } from './subagents'
+import { $subagentsBySession, activeSubagentCount } from './subagents'

 export interface BackgroundResume {
-  /** Latest live activity from the primary child (its newest stream line), or
-   *  null when nothing readable has arrived yet — the UI then falls back to the
-   *  generic "will resume" copy. */
-  activity: string | null
-  /** Running/queued background children for the active session. */
+  /** Running/queued background children belonging to this runtime session. */
  count: number
 }

-const RUNNING = (s: SubagentProgress) => s.status === 'running' || s.status === 'queued'
+/** Background work outlives its spawning turn. The owning view gates this
+ * signal on its own busy state; global focus and raw stream text are unrelated. */
+export function sessionBackgroundResume(sessionId: null | string) {
+  const $count = computed($subagentsBySession, bySession =>
+    sessionId ? activeSubagentCount(bySession[sessionId] ?? []) : 0
+  )

-/**
- * "Parked" background-delegation signal for the active session.
- *
- * A top-level `delegate_task` always runs in the background: the parent turn
- * ends (`$busy` -> false) while the subagent keeps running, and its result
- * re-enters the conversation as a fresh turn when it finishes. During that
- * window the app is genuinely idle but work is still happening elsewhere, so we
- * surface a calm, shimmering status line (its latest activity, or a generic
- * "will resume" fallback) instead of a spinner that reads as "stuck."
- *
- * Null while `$busy`: an active turn already owns the main loader, and subagents
- * spawned inside a running turn (synchronous orchestrator children) are part of
- * that turn, not parked background work the user is waiting on.
- */
-export const $backgroundResume = computed(
-  [$subagentsBySession, $activeSessionId, $busy],
-  (bySession, sid, busy): BackgroundResume | null => {
-    if (busy || !sid) {
-      return null
-    }
-
-    const running = (bySession[sid] ?? []).filter(RUNNING)
-
-    if (running.length === 0) {
-      return null
-    }
-
-    const activity = (running[0]!.stream.at(-1)?.text ?? '').trim() || null
-
-    return { activity, count: running.length }
-  }
-)
+  // Stream frames must not repaint a count-only notice.
+  return computed($count, (count): BackgroundResume | null => (count ? { count } : null))
+}
@@ -9,6 +9,7 @@ import {
  failedSubagentCount,
  pruneDelegateFallbackSubagents,
  pruneFinishedSessionSubagents,
+  reconcileSubagentSnapshot,
  upsertSubagent
 } from './subagents'

@@ -27,6 +28,39 @@ describe('subagent store', () => {
    expect(item?.summary).toBe('done')
  })

+  it('keeps completed children retired across turn pruning, late frames, and roster refreshes', () => {
+    const finished = { subagent_id: 'finished', goal: 'Finished task', status: 'running' }
+    const live = { subagent_id: 'live', goal: 'Background task', status: 'queued' }
+    upsertSubagent('owner', finished, true, 'subagent.start')
+    upsertSubagent('owner', live, true, 'subagent.spawn_requested')
+    upsertSubagent('owner', { ...finished, status: 'completed', summary: 'Done' }, false, 'subagent.complete')
+    upsertSubagent('owner', { ...finished, text: '(°□°) pondering...' }, false, 'subagent.thinking')
+    expect(listFor('owner')[0]?.status).toBe('completed')
+
+    // A completion starts a new parent turn before every delayed child frame
+    // or roster read has drained. Pruning is presentation, not a new child run.
+    pruneFinishedSessionSubagents('owner')
+    const pruned = listFor('owner')
+    reconcileSubagentSnapshot('owner', [finished, live])
+    upsertSubagent('owner', finished, true, 'subagent.start')
+    upsertSubagent('owner', { ...finished, text: '(°□°) pondering...' }, false, 'subagent.thinking')
+    expect(listFor('owner')).toBe(pruned)
+    expect(listFor('owner').map(item => item.id)).toEqual(['live'])
+
+    // Roster-discovered terminal state has the same authority as an event.
+    reconcileSubagentSnapshot('owner', [{ ...live, status: 'interrupted' }])
+    pruneFinishedSessionSubagents('owner')
+    reconcileSubagentSnapshot('owner', [finished, live])
+    expect(activeSubagentCount(listFor('owner'))).toBe(0)
+
+    // Retirement is scoped to this runtime session, not an ID-global ban.
+    upsertSubagent('other', finished, true, 'subagent.start')
+    expect(activeSubagentCount(listFor('other'))).toBe(1)
+    clearSessionSubagents('owner')
+    upsertSubagent('owner', finished, true, 'subagent.start')
+    expect(activeSubagentCount(listFor('owner'))).toBe(1)
+  })
+
  it('builds parent/child trees', () => {
    upsertSubagent('s1', { goal: 'parent', status: 'running', subagent_id: 'p', task_index: 0 })
    upsertSubagent('s1', { goal: 'child', parent_id: 'p', status: 'queued', subagent_id: 'c', task_index: 1 })
@@ -53,6 +53,27 @@ const TOOL_PREVIEW_MAX = 96

 export const $subagentsBySession = atom<Record<string, SubagentProgress[]>>({})

+// A turn prunes display rows, not child identities. Keep retired IDs with the
+// session's current list so late starts/rosters cannot recreate completed work.
+// Clearing the session (or resetting the store) releases this history too.
+const retiredSubagents = new WeakMap<SubagentProgress[], Set<string>>()
+
+function setSessionSubagents(sid: string, previous: SubagentProgress[], next: SubagentProgress[]) {
+  const retired = retiredSubagents.get(previous) ?? new Set<string>()
+
+  for (const item of previous) {
+    if (TERMINAL.has(item.status)) {
+      retired.add(item.id)
+    }
+  }
+
+  if (retired.size) {
+    retiredSubagents.set(next, retired)
+  }
+
+  $subagentsBySession.set({ ...$subagentsBySession.get(), [sid]: next })
+}
+
 const isStr = (v: unknown): v is string => typeof v === 'string'
 const str = (v: unknown) => (isStr(v) ? v : '')
 const num = (v: unknown) => (typeof v === 'number' && Number.isFinite(v) ? v : undefined)
@@ -222,7 +243,7 @@ export function reconcileSubagentSnapshot(sid: string, children: SubagentPayload
  for (const payload of children) {
    const id = str(payload.subagent_id)

-    if (!id) {
+    if (!id || retiredSubagents.get(previous)?.has(id)) {
      continue
    }

@@ -251,7 +272,7 @@ export function reconcileSubagentSnapshot(sid: string, children: SubagentPayload
  }

  if (next.length !== previous.length || next.some((item, index) => item !== previous[index])) {
-    $subagentsBySession.set({ ...map, [sid]: next })
+    setSessionSubagents(sid, previous, next)
  }
 }

@@ -292,7 +313,7 @@ export function pruneFinishedSessionSubagents(sid: string) {
    return
  }

-  $subagentsBySession.set({ ...map, [sid]: next })
+  setSessionSubagents(sid, list, next)
 }

 export function pruneDelegateFallbackSubagents(sid: string) {
@@ -309,7 +330,7 @@ export function pruneDelegateFallbackSubagents(sid: string) {
    return
  }

-  $subagentsBySession.set({ ...map, [sid]: next })
+  setSessionSubagents(sid, list, next)
 }

 export function upsertSubagent(sid: string, payload: SubagentPayload, createIfMissing = true, eventType?: string) {
@@ -318,7 +339,7 @@ export function upsertSubagent(sid: string, payload: SubagentPayload, createIfMi
  const id = idOf(payload)
  const idx = list.findIndex(item => item.id === id)

-  if (idx < 0 && !createIfMissing) {
+  if (retiredSubagents.get(list)?.has(id) || (idx < 0 && !createIfMissing)) {
    return
  }

@@ -331,7 +352,7 @@ export function upsertSubagent(sid: string, payload: SubagentPayload, createIfMi
  const next = toProgress(payload, prev, eventType)
  const nextList = idx >= 0 ? list.map(item => (item.id === id ? next : item)) : [...list, next]

-  $subagentsBySession.set({ ...map, [sid]: nextList })
+  setSessionSubagents(sid, list, nextList)
 }

 export function buildSubagentTree(items: readonly SubagentProgress[]): SubagentNode[] {