From d0567621850ba1a8a7d505f7043588420def5793 Mon Sep 17 00:00:00 2001 From: alari Date: Fri, 12 Jun 2026 11:00:48 +0300 Subject: [PATCH 1/2] fix: make todo panel updates reliable and close it when work stops TaskUpdate calls with ids the server didn't know (taskSeq reset on TodoWrite, empty task map after process restart) were silently dropped, leaving the chat todo panel stale and preventing it from ever reaching all-done. Server now upserts unknown ids, keeps task ids monotonic, honors explicit ids, and seeds restarted processes from the last todo_update in history. Frontend panel is now turn-aware: dismisses 3s after completion once the turn ends (10s while processing), collapses to the pill when a turn ends with unfinished tasks, and detects replaced lists by id signature so a dismissed panel re-shows for new task lists. Co-Authored-By: Claude Opus 4.6 --- server/claude-process.test.ts | 37 ++++++- server/claude-process.ts | 46 +++++++-- server/coding-process.ts | 4 + server/opencode-process.ts | 44 ++++++-- server/session-lifecycle.ts | 12 +++ src/App.tsx | 2 + src/components/OrchestratorContent.tsx | 4 +- src/components/SessionContent.tsx | 4 +- src/components/TodoPanel.test.tsx | 137 +++++++++++++++++++++++++ src/components/TodoPanel.tsx | 46 +++++++-- 10 files changed, 307 insertions(+), 29 deletions(-) create mode 100644 src/components/TodoPanel.test.tsx 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/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(() => { From 3b8b8829b8506ffb603ed32617acfd1ffc1938e3 Mon Sep 17 00:00:00 2001 From: alari Date: Fri, 12 Jun 2026 11:03:51 +0300 Subject: [PATCH 2/2] fix: implement seedTasks no-op in CodexProcess Codex emits the full plan on every turn/plan/updated notification, so there is no incremental task state to seed after a restart. Co-Authored-By: Claude Opus 4.6 --- server/codex-process.ts | 6 ++++++ 1 file changed, 6 insertions(+) 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 }