fix(desktop): clear stale active todos on turn end AND on rehydration
A turn that ends without a final `todo` update left the composer "Tasks N/M" panel pinned with its last item stuck pending/in_progress, and it survived restarts because the panel is read back from stored session history. Two coupled fixes (the first alone is undone by the second path): - Turn end: clear a still-active todo list on `message.complete` and on a terminal `error` (new `clearActiveSessionTodos` — active lists only; a finished list keeps its short linger so the last checkmark still lands). - Rehydration: `hydrateFromStoredSession` runs *after* a turn completes, so an "active" stored list is stale, not in-flight. It now restores only a *finished* list (via new `todosForHydration`) and drops anything still active — otherwise it re-pinned the panel right after the turn-end clear and resurrected it on every restart. Salvages #52996 (@0disoft): the fix shape (clearActiveSessionTodos on turn completion, preserving the finished-list linger) is carried forward and ported onto the current use-message-stream/ folder split (gateway-event.ts), then extended to the rehydration path per review. Co-authored-by: 0disoft <rodisoft1@gmail.com>
This commit is contained in:
parent
e40175f069
commit
3e21cfdebb
5 changed files with 195 additions and 8 deletions
|
|
@ -76,7 +76,7 @@ import {
|
|||
setRememberedSessionId
|
||||
} from '../store/session'
|
||||
import { onSessionsChanged } from '../store/session-sync'
|
||||
import { clearSessionTodos, setSessionTodos, todoListActive } from '../store/todos'
|
||||
import { clearSessionTodos, setSessionTodos, todosForHydration } from '../store/todos'
|
||||
import { openUpdatesWindow, startUpdatePoller, stopUpdatePoller } from '../store/updates'
|
||||
import { isSecondaryWindow } from '../store/windows'
|
||||
|
||||
|
|
@ -484,13 +484,17 @@ export function DesktopController() {
|
|||
storedSessionId
|
||||
)
|
||||
|
||||
// Seed the status stack's todo group from history — but only while
|
||||
// the plan is still in flight, so reopening an old chat doesn't pin
|
||||
// its finished todo list above the composer forever.
|
||||
const todos = latestSessionTodos(messages)
|
||||
// Rehydration runs *after* a turn completes, so an "active" stored
|
||||
// list (last `todo` still pending/in_progress) means the turn ended
|
||||
// without a final update — it's stale, not in-flight. Re-seeding it
|
||||
// would re-pin "Tasks N/M" above the composer and undo the turn-end
|
||||
// clear (and survive restarts, since it's read back from history).
|
||||
// todosForHydration restores only a *finished* list (its short linger
|
||||
// shows the last checkmark); anything still active is dropped.
|
||||
const restored = todosForHydration(latestSessionTodos(messages))
|
||||
|
||||
if (todos && todoListActive(todos)) {
|
||||
setSessionTodos(runtimeSessionId, todos)
|
||||
if (restored) {
|
||||
setSessionTodos(runtimeSessionId, restored)
|
||||
} else {
|
||||
clearSessionTodos(runtimeSessionId)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -37,6 +37,7 @@ import {
|
|||
setYoloActive
|
||||
} from '@/store/session'
|
||||
import { clearSessionSubagents, pruneDelegateFallbackSubagents, upsertSubagent } from '@/store/subagents'
|
||||
import { clearActiveSessionTodos } from '@/store/todos'
|
||||
import { recordToolDiff } from '@/store/tool-diffs'
|
||||
import { notifyWorkspaceChanged, toolMayMutateFiles } from '@/store/workspace-events'
|
||||
import type { RpcEvent } from '@/types/hermes'
|
||||
|
|
@ -308,6 +309,10 @@ export function useGatewayEventHandler(deps: GatewayEventDeps) {
|
|||
// prompt, and vice versa.
|
||||
clearAllPrompts(sessionId)
|
||||
clearClarifyRequest(undefined, sessionId)
|
||||
// Turn ended without a final `todo` update — drop a still-unfinished
|
||||
// list so "Tasks N/M" doesn't stay pinned above the composer with the
|
||||
// last item stuck pending/in_progress. Finished lists keep their linger.
|
||||
clearActiveSessionTodos(sessionId)
|
||||
setSessionCompacting(sessionId, false)
|
||||
|
||||
flushQueuedDeltas(sessionId)
|
||||
|
|
@ -588,6 +593,7 @@ export function useGatewayEventHandler(deps: GatewayEventDeps) {
|
|||
if (sessionId) {
|
||||
clearAllPrompts(sessionId)
|
||||
clearClarifyRequest(undefined, sessionId)
|
||||
clearActiveSessionTodos(sessionId)
|
||||
setSessionCompacting(sessionId, false)
|
||||
compactedTurnRef.current.delete(sessionId)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,93 @@
|
|||
import { QueryClient } from '@tanstack/react-query'
|
||||
import { act, cleanup, render, waitFor } from '@testing-library/react'
|
||||
import { useEffect, useRef } from 'react'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import type { ClientSessionState } from '@/app/types'
|
||||
import { createClientSessionState } from '@/lib/chat-runtime'
|
||||
import type { TodoItem } from '@/lib/todos'
|
||||
import { $todosBySession, clearSessionTodos, setSessionTodos } from '@/store/todos'
|
||||
import type { RpcEvent } from '@/types/hermes'
|
||||
|
||||
import { useMessageStream } from './index'
|
||||
|
||||
const SID = 'session-1'
|
||||
const todo = (id: string, status: TodoItem['status']): TodoItem => ({ content: `task ${id}`, id, status })
|
||||
|
||||
let handleEvent: ((event: RpcEvent) => void) | null = null
|
||||
|
||||
function Harness() {
|
||||
const activeSessionIdRef = useRef<string | null>(SID)
|
||||
const sessionStateByRuntimeIdRef = useRef(new Map<string, ClientSessionState>())
|
||||
const queryClientRef = useRef(new QueryClient())
|
||||
|
||||
const stream = useMessageStream({
|
||||
activeSessionIdRef,
|
||||
hydrateFromStoredSession: vi.fn(async () => undefined),
|
||||
queryClient: queryClientRef.current,
|
||||
refreshHermesConfig: vi.fn(async () => undefined),
|
||||
refreshSessions: vi.fn(async () => undefined),
|
||||
sessionStateByRuntimeIdRef,
|
||||
updateSessionState: (sessionId, updater) => {
|
||||
const current = sessionStateByRuntimeIdRef.current.get(sessionId) ?? createClientSessionState()
|
||||
const next = updater(current)
|
||||
sessionStateByRuntimeIdRef.current.set(sessionId, next)
|
||||
|
||||
return next
|
||||
}
|
||||
})
|
||||
|
||||
useEffect(() => {
|
||||
handleEvent = stream.handleGatewayEvent
|
||||
}, [stream.handleGatewayEvent])
|
||||
|
||||
return null
|
||||
}
|
||||
|
||||
async function mountStream() {
|
||||
render(<Harness />)
|
||||
await waitFor(() => expect(handleEvent).not.toBeNull())
|
||||
}
|
||||
|
||||
const complete = () => act(() => handleEvent!({ payload: { text: 'done' }, session_id: SID, type: 'message.complete' }))
|
||||
|
||||
describe('useMessageStream turn-end todo cleanup', () => {
|
||||
beforeEach(() => {
|
||||
handleEvent = null
|
||||
clearSessionTodos(SID)
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
cleanup()
|
||||
clearSessionTodos(SID)
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
|
||||
it('drops a still-active task list when the turn completes', async () => {
|
||||
await mountStream()
|
||||
setSessionTodos(SID, [todo('a', 'completed'), todo('b', 'in_progress')])
|
||||
|
||||
complete()
|
||||
|
||||
expect($todosBySession.get()[SID]).toBeUndefined()
|
||||
})
|
||||
|
||||
it('keeps a finished list on completion so its linger shows the final checkmarks', async () => {
|
||||
await mountStream()
|
||||
setSessionTodos(SID, [todo('a', 'completed')])
|
||||
|
||||
complete()
|
||||
|
||||
// Not cleared immediately — the finished-list linger still owns it.
|
||||
expect($todosBySession.get()[SID]).toHaveLength(1)
|
||||
})
|
||||
|
||||
it('drops a still-active task list when the turn errors out', async () => {
|
||||
await mountStream()
|
||||
setSessionTodos(SID, [todo('a', 'in_progress')])
|
||||
|
||||
act(() => handleEvent!({ payload: { message: 'boom' }, session_id: SID, type: 'error' }))
|
||||
|
||||
expect($todosBySession.get()[SID]).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
|
@ -2,7 +2,13 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
|||
|
||||
import type { TodoItem } from '@/lib/todos'
|
||||
|
||||
import { $todosBySession, clearSessionTodos, setSessionTodos } from './todos'
|
||||
import {
|
||||
$todosBySession,
|
||||
clearActiveSessionTodos,
|
||||
clearSessionTodos,
|
||||
setSessionTodos,
|
||||
todosForHydration
|
||||
} from './todos'
|
||||
|
||||
const todo = (id: string, status: TodoItem['status']): TodoItem => ({ content: `task ${id}`, id, status })
|
||||
|
||||
|
|
@ -45,3 +51,55 @@ describe('setSessionTodos finished-list auto-clear', () => {
|
|||
expect($todosBySession.get().s1).toHaveLength(2)
|
||||
})
|
||||
})
|
||||
|
||||
describe('clearActiveSessionTodos (turn-end cleanup)', () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
clearSessionTodos('s1')
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
it('drops a still-active list when the turn has ended', () => {
|
||||
setSessionTodos('s1', [todo('a', 'completed'), todo('b', 'in_progress')])
|
||||
|
||||
clearActiveSessionTodos('s1')
|
||||
|
||||
expect($todosBySession.get().s1).toBeUndefined()
|
||||
})
|
||||
|
||||
it('leaves a finished list to its normal linger instead of clearing immediately', () => {
|
||||
setSessionTodos('s1', [todo('a', 'completed')])
|
||||
|
||||
clearActiveSessionTodos('s1')
|
||||
|
||||
expect($todosBySession.get().s1).toHaveLength(1)
|
||||
vi.advanceTimersByTime(5_000)
|
||||
expect($todosBySession.get().s1).toBeUndefined()
|
||||
})
|
||||
|
||||
it('is a no-op when the session has no todos', () => {
|
||||
clearActiveSessionTodos('s1')
|
||||
|
||||
expect($todosBySession.get().s1).toBeUndefined()
|
||||
})
|
||||
})
|
||||
|
||||
describe('todosForHydration (stale-active guard on restore)', () => {
|
||||
it('does not restore an active list (stale after a completed turn)', () => {
|
||||
expect(todosForHydration([todo('a', 'completed'), todo('b', 'in_progress')])).toBeNull()
|
||||
expect(todosForHydration([todo('a', 'pending')])).toBeNull()
|
||||
})
|
||||
|
||||
it('restores a finished list so its linger shows the final checkmarks', () => {
|
||||
const finished = [todo('a', 'completed'), todo('b', 'cancelled')]
|
||||
|
||||
expect(todosForHydration(finished)).toEqual(finished)
|
||||
})
|
||||
|
||||
it('returns null when there is nothing stored', () => {
|
||||
expect(todosForHydration(null)).toBeNull()
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -16,6 +16,17 @@ export const $todosBySession = atom<Record<string, TodoItem[]>>({})
|
|||
export const todoListActive = (todos: readonly TodoItem[]) =>
|
||||
todos.some(t => t.status === 'pending' || t.status === 'in_progress')
|
||||
|
||||
// Decide which todo list to restore when rehydrating a session from stored
|
||||
// history. Rehydration runs *after* a turn completes, so an active list (last
|
||||
// item still pending/in_progress) is stale — the turn ended without a final
|
||||
// `todo` update — and must NOT be re-pinned (that would undo the turn-end
|
||||
// clear and, because it's read back from history, resurrect on restart). Only
|
||||
// a finished list is restored, so its short linger shows the last checkmark.
|
||||
// Returns null when there's nothing to restore (caller should clear).
|
||||
export function todosForHydration(todos: readonly TodoItem[] | null): TodoItem[] | null {
|
||||
return todos && !todoListActive(todos) ? [...todos] : null
|
||||
}
|
||||
|
||||
// Once a list finishes (every item completed/cancelled), the final state
|
||||
// lingers just long enough to see the last checkmark land, then the group
|
||||
// drops out of the stack on its own.
|
||||
|
|
@ -62,3 +73,18 @@ export function clearSessionTodos(sid: string) {
|
|||
const { [sid]: _drop, ...rest } = map
|
||||
$todosBySession.set(rest)
|
||||
}
|
||||
|
||||
// Drop a still-active todo list (any pending/in_progress item) — used at turn
|
||||
// end, when an unfinished list means the turn stopped without a final `todo`
|
||||
// update, so the "Tasks N/M" panel would otherwise stay pinned above the
|
||||
// composer forever. A finished list is left untouched so its short linger
|
||||
// still shows the last checkmark landing.
|
||||
export function clearActiveSessionTodos(sid: string) {
|
||||
const todos = $todosBySession.get()[sid]
|
||||
|
||||
if (!todos || !todoListActive(todos)) {
|
||||
return
|
||||
}
|
||||
|
||||
clearSessionTodos(sid)
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue