diff --git a/server/claude-process.test.ts b/server/claude-process.test.ts index 58d9cea4..468caf4d 100644 --- a/server/claude-process.test.ts +++ b/server/claude-process.test.ts @@ -133,14 +133,16 @@ describe('handleTaskTool', () => { expect(cp.tasks.size).toBe(1) }) - it('clears existing tasks and resets ID sequence', () => { + it('clears existing tasks but keeps the ID sequence monotonic', () => { cp.handleTaskTool('TaskCreate', { subject: 'Old task' }) expect(cp.tasks.size).toBe(1) cp.handleTaskTool('TodoWrite', { todos: [{ content: 'New', status: 'in_progress' }], }) expect(cp.tasks.size).toBe(1) - expect(cp.tasks.get('1')!.subject).toBe('New') + // New list gets a fresh id (not reused '1') so the frontend can detect + // a replaced list and later TaskCreate/TaskUpdate ids never collide. + expect(cp.tasks.get('2')!.subject).toBe('New') }) it('returns false for non-array todos', () => { @@ -182,8 +184,18 @@ describe('handleTaskTool', () => { expect(cp.tasks.has('1')).toBe(false) }) - it('returns false for nonexistent task', () => { - expect(cp.handleTaskTool('TaskUpdate', { taskId: '999', status: 'completed' })).toBe(false) + it('upserts a task for an unknown id instead of dropping the update', () => { + expect(cp.handleTaskTool('TaskUpdate', { taskId: '999', status: 'completed' })).toBe(true) + expect(cp.tasks.get('999')!.status).toBe('completed') + expect(cp.tasks.get('999')!.subject).toBe('Task 999') + }) + + it('returns false for unknown id with deleted status', () => { + expect(cp.handleTaskTool('TaskUpdate', { taskId: '999', status: 'deleted' })).toBe(false) + }) + + it('returns false for missing taskId', () => { + expect(cp.handleTaskTool('TaskUpdate', { status: 'completed' })).toBe(false) }) it('updates subject and activeForm', () => { @@ -196,6 +208,23 @@ describe('handleTaskTool', () => { it('returns false for unknown tool', () => { expect(cp.handleTaskTool('UnknownTool', {})).toBe(false) }) + + describe('seedTasks', () => { + it('restores tasks and continues the id sequence past seeded ids', () => { + cp.seedTasks([ + { id: '3', subject: 'Restored A', status: 'completed' }, + { id: '4', subject: 'Restored B', status: 'in_progress' }, + ]) + expect(cp.tasks.size).toBe(2) + // TaskUpdate against a pre-restart id now resolves to the real task + cp.handleTaskTool('TaskUpdate', { taskId: '4', status: 'completed' }) + expect(cp.tasks.get('4')!.status).toBe('completed') + expect(cp.tasks.get('4')!.subject).toBe('Restored B') + // New TaskCreate ids do not collide with seeded ids + cp.handleTaskTool('TaskCreate', { subject: 'Next' }) + expect(cp.tasks.get('5')!.subject).toBe('Next') + }) + }) }) describe('handleStreamEvent (via handleLine)', () => { diff --git a/server/claude-process.ts b/server/claude-process.ts index 17955c9a..3196a895 100644 --- a/server/claude-process.ts +++ b/server/claude-process.ts @@ -486,7 +486,9 @@ export class ClaudeProcess extends EventEmitter implements if (isTask && TOOL_DEBUG) console.log('[task-debug] tool:', this.tool.name, 'input:', JSON.stringify(parsed).slice(0, 200)) if (this.handleTaskTool(this.tool.name!, parsed)) { if (TOOL_DEBUG) console.log('[task-debug] emitting todo_update, tasks:', this.tasks.size) - this.emit('todo_update', Array.from(this.tasks.values())) + // Copy items so later in-place TaskUpdate mutations don't alias into + // previously broadcast/history-stored snapshots. + this.emit('todo_update', Array.from(this.tasks.values(), t => ({ ...t }))) } } catch (err) { console.warn(`[claude] Failed to parse tool input for ${this.tool.name}:`, err instanceof Error ? err.message : err) @@ -648,14 +650,17 @@ export class ClaudeProcess extends EventEmitter implements * Returns true if the task list changed (caller should emit todo_update). */ private handleTaskTool(toolName: string, input: Record): boolean { - // TodoWrite sends the entire list at once + // TodoWrite sends the entire list at once. + // Note: taskSeq is intentionally NOT reset — keeping ids monotonic across + // list generations lets the frontend detect a brand-new list by id and + // avoids id collisions with later TaskCreate/TaskUpdate calls. if (toolName === 'TodoWrite') { const todos = input.todos as Array> | undefined if (!Array.isArray(todos)) return false this.tasks.clear() - this.taskSeq = 0 for (const item of todos) { const id = String(item.id || ++this.taskSeq) + this.syncTaskSeq(id) const status = item.status as string if (status !== 'pending' && status !== 'in_progress' && status !== 'completed') continue this.tasks.set(id, { @@ -669,7 +674,8 @@ export class ClaudeProcess extends EventEmitter implements } // TaskCreate/TaskUpdate are the newer tool names if (toolName === 'TaskCreate') { - const id = String(++this.taskSeq) + const id = String(input.taskId || input.id || ++this.taskSeq) + this.syncTaskSeq(id) this.tasks.set(id, { id, subject: String(input.subject || ''), @@ -680,8 +686,17 @@ export class ClaudeProcess extends EventEmitter implements } if (toolName === 'TaskUpdate') { const id = String(input.taskId || '') - const task = this.tasks.get(id) - if (!task) return false + if (!id) return false + let task = this.tasks.get(id) + if (!task) { + // Unknown id — our in-memory map can diverge from the CLI's real task + // list (e.g. after a process restart mid-session). Upsert instead of + // dropping the update, otherwise the UI shows a stale list forever. + if (input.status === 'deleted') return false + task = { id, subject: String(input.subject || `Task ${id}`), status: 'pending' } + this.tasks.set(id, task) + this.syncTaskSeq(id) + } const status = input.status as string | undefined if (status === 'deleted') { this.tasks.delete(id) @@ -697,6 +712,25 @@ export class ClaudeProcess extends EventEmitter implements return false } + /** Keep taskSeq ahead of any numeric id we have seen, so generated ids never collide. */ + private syncTaskSeq(id: string): void { + const n = Number(id) + if (Number.isInteger(n) && n > this.taskSeq) this.taskSeq = n + } + + /** + * Seed task state from a previous process's last known list (session restore). + * Without this, a restarted process starts with an empty map and TaskUpdate + * calls referencing pre-restart task ids would otherwise be lost. + */ + seedTasks(tasks: TaskItem[]): void { + this.tasks.clear() + for (const t of tasks) { + this.tasks.set(t.id, { ...t }) + this.syncTaskSeq(t.id) + } + } + /** * Extract a short summary from extended thinking text. * Tries to grab the first sentence (up to 120 chars), or truncates at a diff --git a/server/codex-process.ts b/server/codex-process.ts index 3069d454..515370e3 100644 --- a/server/codex-process.ts +++ b/server/codex-process.ts @@ -805,6 +805,12 @@ export class CodexProcess extends EventEmitter implements C } } + /** + * No-op: Codex emits the full plan on every turn/plan/updated notification, + * so there is no incremental task state to seed after a restart. + */ + seedTasks(): void {} + isAlive(): boolean { return this.alive } diff --git a/server/coding-process.ts b/server/coding-process.ts index 97e18f20..6c3ee560 100644 --- a/server/coding-process.ts +++ b/server/coding-process.ts @@ -12,6 +12,7 @@ import type { EventEmitter } from 'events' import type { ClaudeProcessEvents } from './claude-process.js' +import type { TaskItem } from './types.js' /** * Supported AI coding assistant providers. @@ -81,6 +82,9 @@ export interface CodingProcess extends EventEmitter { */ setPermissionMode?(mode: import('./types.js').PermissionMode): Promise + /** Seed task/todo state from a previous process's last known list (session restore). */ + seedTasks(tasks: TaskItem[]): void + /** Whether the process is currently running and accepting input. */ isAlive(): boolean diff --git a/server/opencode-process.ts b/server/opencode-process.ts index 58897718..116089d2 100644 --- a/server/opencode-process.ts +++ b/server/opencode-process.ts @@ -889,13 +889,13 @@ export class OpenCodeProcess extends EventEmitter implement this.emit('tool_active', toolName, inputStr) // Detect task/todo tool calls and emit todo_update if (part.state?.input && this.handleTaskTool(toolName, part.state.input)) { - this.emit('todo_update', Array.from(this.tasks.values())) + this.emit('todo_update', Array.from(this.tasks.values(), t => ({ ...t }))) } } else if (status === 'completed') { // Also check for task tools at completion (some providers only // populate input at this stage, not during 'running') if (part.state?.input && this.handleTaskTool(toolName, part.state.input)) { - this.emit('todo_update', Array.from(this.tasks.values())) + this.emit('todo_update', Array.from(this.tasks.values(), t => ({ ...t }))) } const output = part.state?.output const summary = output ? output.slice(0, 200) : undefined @@ -1265,13 +1265,16 @@ export class OpenCodeProcess extends EventEmitter implement private handleTaskTool(toolName: string, input: Record): boolean { // Normalize tool name — OpenCode may report as 'todowrite', 'TodoWrite', 'todo_write', etc. const normalized = toolName.toLowerCase().replace(/_/g, '') + // Note: taskSeq is intentionally NOT reset on TodoWrite — keeping ids + // monotonic across list generations lets the frontend detect a brand-new + // list by id and avoids collisions with later TaskCreate/TaskUpdate calls. if (normalized === 'todowrite') { const todos = input.todos as Array> | undefined if (!Array.isArray(todos)) return false this.tasks.clear() - this.taskSeq = 0 for (const item of todos) { const id = String(item.id || ++this.taskSeq) + this.syncTaskSeq(id) const status = item.status as string if (status !== 'pending' && status !== 'in_progress' && status !== 'completed') continue this.tasks.set(id, { @@ -1284,7 +1287,8 @@ export class OpenCodeProcess extends EventEmitter implement return true } if (normalized === 'taskcreate') { - const id = String(++this.taskSeq) + const id = String(input.taskId || input.id || ++this.taskSeq) + this.syncTaskSeq(id) this.tasks.set(id, { id, subject: String(input.subject || ''), @@ -1295,8 +1299,17 @@ export class OpenCodeProcess extends EventEmitter implement } if (normalized === 'taskupdate') { const id = String(input.taskId || '') - const task = this.tasks.get(id) - if (!task) return false + if (!id) return false + let task = this.tasks.get(id) + if (!task) { + // Unknown id — our in-memory map can diverge from the provider's real + // task list (e.g. after a process restart mid-session). Upsert instead + // of dropping the update, otherwise the UI shows a stale list forever. + if (input.status === 'deleted') return false + task = { id, subject: String(input.subject || `Task ${id}`), status: 'pending' } + this.tasks.set(id, task) + this.syncTaskSeq(id) + } const status = input.status as string | undefined if (status === 'deleted') { this.tasks.delete(id) @@ -1312,6 +1325,25 @@ export class OpenCodeProcess extends EventEmitter implement return false } + /** Keep taskSeq ahead of any numeric id we have seen, so generated ids never collide. */ + private syncTaskSeq(id: string): void { + const n = Number(id) + if (Number.isInteger(n) && n > this.taskSeq) this.taskSeq = n + } + + /** + * Seed task state from a previous process's last known list (session restore). + * Without this, a restarted process starts with an empty map and TaskUpdate + * calls referencing pre-restart task ids would otherwise be lost. + */ + seedTasks(tasks: TaskItem[]): void { + this.tasks.clear() + for (const t of tasks) { + this.tasks.set(t.id, { ...t }) + this.syncTaskSeq(t.id) + } + } + /** Send a user message to the OpenCode session. */ sendMessage(content: string): void { if (!this.alive || !this.opencodeSessionId) { diff --git a/server/session-lifecycle.ts b/server/session-lifecycle.ts index 10066dc1..a04ba5ae 100644 --- a/server/session-lifecycle.ts +++ b/server/session-lifecycle.ts @@ -188,6 +188,18 @@ export class SessionLifecycle { this.wireClaudeEvents(cp, session, sessionId) + // On resume, carry the last known task list into the new process so + // TaskUpdate calls referencing pre-restart task ids aren't dropped. + if (resume) { + for (let i = session.outputHistory.length - 1; i >= 0; i--) { + const msg = session.outputHistory[i] + if (msg.type === 'todo_update') { + cp.seedTasks(msg.tasks) + break + } + } + } + cp.start() session.claudeProcess = cp diff --git a/src/App.tsx b/src/App.tsx index f047bfaa..d9914520 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -666,6 +666,7 @@ export default function App() { planningMode={planningMode} activityLabel={activityLabel} tasks={tasks} + isProcessing={isProcessing} activePrompt={activePrompt} sendPromptResponse={sendPromptResponse} inputBarRef={inputBarRef} @@ -733,6 +734,7 @@ export default function App() { planningMode={planningMode} activityLabel={activityLabel} tasks={tasks} + isProcessing={isProcessing} disabled={!settings.token} hasFileChanges={hasFileChanges} diffPanelOpen={diffPanelOpen} diff --git a/src/components/OrchestratorContent.tsx b/src/components/OrchestratorContent.tsx index a5f8f146..4d81b45f 100644 --- a/src/components/OrchestratorContent.tsx +++ b/src/components/OrchestratorContent.tsx @@ -27,6 +27,7 @@ export interface OrchestratorContentProps { planningMode: boolean activityLabel?: string tasks: TaskItem[] + isProcessing: boolean activePrompt: PromptEntry | null sendPromptResponse: (value: string | string[], requestId?: string) => void inputBarRef: RefObject @@ -55,6 +56,7 @@ export function OrchestratorContent({ planningMode, activityLabel, tasks, + isProcessing, activePrompt, sendPromptResponse, inputBarRef, @@ -93,7 +95,7 @@ export function OrchestratorContent({ variant="orchestrator" agentName={agentName} /> - + {activePrompt && ( - + {/* Claude Code disabled banner */} {claudeDisabled && ( diff --git a/src/components/TodoPanel.test.tsx b/src/components/TodoPanel.test.tsx new file mode 100644 index 00000000..a1280209 --- /dev/null +++ b/src/components/TodoPanel.test.tsx @@ -0,0 +1,137 @@ +/** Tests for TodoPanel — verifies show/hide/collapse behavior across task list updates and turn boundaries. */ +// @vitest-environment jsdom +;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' +import { act } from 'react' +import { createRoot } from 'react-dom/client' +import { TodoPanel } from './TodoPanel.js' +import type { TaskItem } from '../types' + +/* ------------------------------------------------------------------ */ +/* Minimal component renderer */ +/* ------------------------------------------------------------------ */ + +let activeRoot: ReturnType | null = null +let activeContainer: HTMLElement | null = null + +function render(ui: React.ReactElement): HTMLElement { + const container = document.createElement('div') + document.body.appendChild(container) + act(() => { + activeRoot = createRoot(container) + activeRoot.render(ui) + }) + activeContainer = container + return container +} + +function rerender(ui: React.ReactElement) { + act(() => { + activeRoot!.render(ui) + }) +} + +afterEach(() => { + if (activeRoot) act(() => activeRoot!.unmount()) + activeContainer?.remove() + activeRoot = null + activeContainer = null +}) + +beforeEach(() => { + vi.useFakeTimers() +}) + +afterEach(() => { + vi.useRealTimers() +}) + +/* ------------------------------------------------------------------ */ +/* Helpers */ +/* ------------------------------------------------------------------ */ + +function task(id: string, status: TaskItem['status'], subject = `Task ${id}`): TaskItem { + return { id, subject, status } +} + +function isVisible(container: HTMLElement): boolean { + return container.children.length > 0 +} + +function isExpanded(container: HTMLElement): boolean { + return container.textContent?.includes('Tasks') ?? false +} + +/* ------------------------------------------------------------------ */ +/* Tests */ +/* ------------------------------------------------------------------ */ + +describe('TodoPanel', () => { + it('renders nothing with no tasks', () => { + const c = render() + expect(isVisible(c)).toBe(false) + }) + + it('auto-expands when new tasks arrive while processing', () => { + const c = render() + rerender() + expect(isVisible(c)).toBe(true) + expect(isExpanded(c)).toBe(true) + }) + + it('shows a collapsed pill (not expanded) when restoring an unfinished list while idle', () => { + const c = render() + expect(isVisible(c)).toBe(true) + expect(isExpanded(c)).toBe(false) + }) + + it('auto-dismisses 3s after all tasks complete once the turn is over', () => { + const c = render() + rerender() + expect(isVisible(c)).toBe(true) + act(() => { vi.advanceTimersByTime(3000) }) + expect(isVisible(c)).toBe(false) + }) + + it('keeps the panel for 10s after completion while still processing', () => { + const c = render() + rerender() + act(() => { vi.advanceTimersByTime(3000) }) + expect(isVisible(c)).toBe(true) + act(() => { vi.advanceTimersByTime(7000) }) + expect(isVisible(c)).toBe(false) + }) + + it('collapses to the pill when a turn ends with unfinished tasks', () => { + const c = render() + rerender() + expect(isExpanded(c)).toBe(true) + rerender() + expect(isVisible(c)).toBe(true) + expect(isExpanded(c)).toBe(false) + }) + + it('re-shows a dismissed panel when a replaced list with new ids arrives (same length)', () => { + const c = render() + rerender() + // complete + dismiss + rerender() + act(() => { vi.advanceTimersByTime(3000) }) + expect(isVisible(c)).toBe(false) + // new list generation, same length but different id + rerender() + expect(isVisible(c)).toBe(true) + }) + + it('dismisses immediately when restoring an already-completed list', () => { + const c = render() + act(() => { vi.advanceTimersByTime(0) }) + expect(isVisible(c)).toBe(false) + }) + + it('updates the progress count in the pill', () => { + const c = render() + expect(c.textContent).toContain('1/2') + }) +}) diff --git a/src/components/TodoPanel.tsx b/src/components/TodoPanel.tsx index b6967c22..ccb9cd19 100644 --- a/src/components/TodoPanel.tsx +++ b/src/components/TodoPanel.tsx @@ -12,28 +12,40 @@ import type { TaskItem } from '../types' interface Props { tasks: TaskItem[] + /** Whether the session is currently processing a turn. Used to close/collapse the panel when work stops. */ + isProcessing?: boolean } -export function TodoPanel({ tasks }: Props) { +export function TodoPanel({ tasks, isProcessing = false }: Props) { const [expanded, setExpanded] = useState(false) const [dismissed, setDismissed] = useState(false) - const prevCountRef = useRef(0) + const prevIdSigRef = useRef('') const hadActiveTaskRef = useRef(false) const prevCompletedRef = useRef(0) + const prevProcessingRef = useRef(isProcessing) // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-arguments -- new Map() infers Map const taskRefs = useRef>(new Map()) const completed = tasks.filter(t => t.status === 'completed').length const allDone = tasks.length > 0 && completed === tasks.length + // Signature of task identity — detects replaced lists, not just count growth + const idSig = tasks.map(t => t.id).join(',') - // Auto-expand when new tasks appear + // Auto-show when a new or replaced task list with active work appears. + // Compares task ids (not just count) so a fresh list of equal/smaller size + // still re-shows a previously dismissed panel. useEffect(() => { - if (tasks.length > prevCountRef.current) { - setExpanded(true) // eslint-disable-line react-hooks/set-state-in-effect -- auto-expand on new tasks - setDismissed(false) + if (idSig !== prevIdSigRef.current) { + prevIdSigRef.current = idSig + if (idSig && !allDone) { + /* eslint-disable react-hooks/set-state-in-effect -- auto-show on new task list */ + setDismissed(false) + // Expand only while actively processing; on idle restore show the pill + if (isProcessing) setExpanded(true) + /* eslint-enable react-hooks/set-state-in-effect */ + } } - prevCountRef.current = tasks.length - }, [tasks.length]) + }, [idSig, allDone, isProcessing]) // Track if we ever saw a non-completed task (distinguishes live vs restore) useEffect(() => { @@ -42,16 +54,28 @@ export function TodoPanel({ tasks }: Props) { } }, [tasks, allDone]) - // Auto-dismiss after all tasks complete + // Auto-dismiss after all tasks complete: quickly once the turn is over, + // with a longer grace period while Claude is still working. useEffect(() => { if (allDone) { - const delay = hadActiveTaskRef.current ? 10000 : 0 + const delay = !hadActiveTaskRef.current ? 0 : isProcessing ? 10000 : 3000 const timer = setTimeout(() => { setDismissed(true); }, delay) return () => { clearTimeout(timer); } } setDismissed(false) // eslint-disable-line react-hooks/set-state-in-effect -- reset dismissed when tasks become active return undefined - }, [allDone]) + }, [allDone, isProcessing]) + + // When a turn ends with unfinished tasks (Claude often forgets the final + // update), collapse the expanded card to the compact pill so it stops + // obstructing the chat while still showing progress. + useEffect(() => { + const wasProcessing = prevProcessingRef.current + prevProcessingRef.current = isProcessing + if (wasProcessing && !isProcessing && tasks.length > 0 && !allDone) { + setExpanded(false) // eslint-disable-line react-hooks/set-state-in-effect -- collapse on turn end + } + }, [isProcessing, tasks.length, allDone]) // Auto-scroll to first non-completed task when completed count increases useEffect(() => {