diff --git a/packages/types/src/message.ts b/packages/types/src/message.ts index 109cd842bac..dc7d8f23448 100644 --- a/packages/types/src/message.ts +++ b/packages/types/src/message.ts @@ -180,6 +180,7 @@ export const clineSays = [ "sliding_window_truncation", "codebase_search_result", "user_edit_todos", + "system_update_todos", ] as const export const clineSaySchema = z.enum(clineSays) diff --git a/packages/types/src/todo.ts b/packages/types/src/todo.ts index 4e874e17505..0530f920542 100644 --- a/packages/types/src/todo.ts +++ b/packages/types/src/todo.ts @@ -14,6 +14,10 @@ export const todoItemSchema = z.object({ id: z.string(), content: z.string(), status: todoStatusSchema, + // Optional fields for subtask tracking + subtaskId: z.string().optional(), // ID of the linked subtask (child task) for direct cost/token attribution + tokens: z.number().optional(), // Total tokens (in + out) for linked subtask + cost: z.number().optional(), // Total cost for linked subtask }) export type TodoItem = z.infer diff --git a/packages/types/src/vscode-extension-host.ts b/packages/types/src/vscode-extension-host.ts index f3116141f04..1448c08c809 100644 --- a/packages/types/src/vscode-extension-host.ts +++ b/packages/types/src/vscode-extension-host.ts @@ -188,6 +188,14 @@ export interface ExtensionMessage { totalCost: number ownCost: number childrenCost: number + childDetails?: { + id: string + name: string + tokens: number + cost: number + status: "active" | "completed" | "delegated" + hasNestedChildren: boolean + }[] } historyItem?: HistoryItem } diff --git a/src/__tests__/history-resume-delegation.spec.ts b/src/__tests__/history-resume-delegation.spec.ts index 1f95d0f6dde..233e7ece158 100644 --- a/src/__tests__/history-resume-delegation.spec.ts +++ b/src/__tests__/history-resume-delegation.spec.ts @@ -3,6 +3,13 @@ import { describe, it, expect, vi, beforeEach } from "vitest" import { RooCodeEventName } from "@roo-code/types" +// Keep AttemptCompletionTool tests deterministic (TelemetryService can be undefined in unit test env) +vi.mock("@roo-code/telemetry", () => ({ + TelemetryService: { + instance: { captureTaskCompleted: vi.fn() }, + }, +})) + /* vscode mock for Task/Provider imports */ vi.mock("vscode", () => { const window = { @@ -374,13 +381,15 @@ describe("History resume delegation - parent metadata transitions", () => { }) // Verify both events emitted - const eventNames = emitSpy.mock.calls.map((c) => c[0]) + const eventNames = emitSpy.mock.calls.map((c: any[]) => c[0]) expect(eventNames).toContain(RooCodeEventName.TaskDelegationCompleted) expect(eventNames).toContain(RooCodeEventName.TaskDelegationResumed) // CRITICAL: verify ordering (TaskDelegationCompleted before TaskDelegationResumed) - const completedIdx = emitSpy.mock.calls.findIndex((c) => c[0] === RooCodeEventName.TaskDelegationCompleted) - const resumedIdx = emitSpy.mock.calls.findIndex((c) => c[0] === RooCodeEventName.TaskDelegationResumed) + const completedIdx = emitSpy.mock.calls.findIndex( + (c: any[]) => c[0] === RooCodeEventName.TaskDelegationCompleted, + ) + const resumedIdx = emitSpy.mock.calls.findIndex((c: any[]) => c[0] === RooCodeEventName.TaskDelegationResumed) expect(completedIdx).toBeGreaterThanOrEqual(0) expect(resumedIdx).toBeGreaterThan(completedIdx) }) @@ -424,7 +433,7 @@ describe("History resume delegation - parent metadata transitions", () => { }) // CRITICAL: verify legacy pause/unpause events NOT emitted - const eventNames = emitSpy.mock.calls.map((c) => c[0]) + const eventNames = emitSpy.mock.calls.map((c: any[]) => c[0]) expect(eventNames).not.toContain(RooCodeEventName.TaskPaused) expect(eventNames).not.toContain(RooCodeEventName.TaskUnpaused) expect(eventNames).not.toContain(RooCodeEventName.TaskSpawned) @@ -491,4 +500,278 @@ describe("History resume delegation - parent metadata transitions", () => { }), ) }) + + it("reopenParentFromDelegation uses fallback anchor when subtaskId link is missing but child is valid", async () => { + const provider = { + contextProxy: { globalStorageUri: { fsPath: "/storage" } }, + getTaskWithId: vi.fn().mockImplementation((taskId: string) => { + if (taskId === "parent-fallback") { + return Promise.resolve({ + historyItem: { + id: "parent-fallback", + status: "delegated", + awaitingChildId: "child-fallback", + childIds: ["child-fallback"], // This validates the parent-child relationship + ts: 100, + task: "Parent task", + tokensIn: 0, + tokensOut: 0, + totalCost: 0, + }, + }) + } + // Child history item with tokens/cost + return Promise.resolve({ + historyItem: { + id: "child-fallback", + tokensIn: 500, + tokensOut: 300, + totalCost: 0.05, + ts: 200, + task: "Child task", + }, + }) + }), + emit: vi.fn(), + getCurrentTask: vi.fn(() => ({ taskId: "child-fallback" })), + removeClineFromStack: vi.fn().mockResolvedValue(undefined), + createTaskWithHistoryItem: vi.fn().mockResolvedValue({ + taskId: "parent-fallback", + resumeAfterDelegation: vi.fn().mockResolvedValue(undefined), + }), + updateTaskHistory: vi.fn().mockResolvedValue([]), + } as unknown as ClineProvider + + // Parent has all completed todos but NO subtaskId link + const parentMessagesWithCompletedTodos = [ + { + type: "say", + say: "system_update_todos", + text: JSON.stringify({ + tool: "updateTodoList", + todos: [ + { id: "todo-1", content: "First completed", status: "completed" }, + { id: "todo-2", content: "Second completed", status: "completed" }, + { id: "todo-3", content: "Last completed", status: "completed" }, + // Note: NO subtaskId on any todo - this is the bug scenario + ], + }), + ts: 50, + }, + ] + + vi.mocked(readTaskMessages).mockResolvedValue(parentMessagesWithCompletedTodos as any) + vi.mocked(readApiMessages).mockResolvedValue([]) + + await (ClineProvider.prototype as any).reopenParentFromDelegation.call(provider, { + parentTaskId: "parent-fallback", + childTaskId: "child-fallback", + completionResultSummary: "Child completed successfully", + }) + + // Verify that saveTaskMessages was called and includes the todo write-back + expect(saveTaskMessages).toHaveBeenCalled() + const savedCall = vi.mocked(saveTaskMessages).mock.calls[0][0] + + // Find the system_update_todos message that was added for the write-back + const todoEditMessages = savedCall.messages.filter( + (m: any) => m.type === "say" && m.say === "system_update_todos", + ) + + // Should have at least 2 todo edit messages (original + write-back) + expect(todoEditMessages.length).toBeGreaterThanOrEqual(1) + + // Parse the last todo edit to verify fallback worked + const lastTodoEdit = todoEditMessages[todoEditMessages.length - 1] + expect(lastTodoEdit.text).toBeDefined() + const parsedTodos = JSON.parse(lastTodoEdit.text as string) + + // The LAST completed todo should have been selected as the fallback anchor + // and should now have subtaskId, tokens, and cost + const anchoredTodo = parsedTodos.todos.find((t: any) => t.subtaskId === "child-fallback") + expect(anchoredTodo).toBeDefined() + expect(anchoredTodo.content).toBe("Last completed") // Fallback picks LAST completed + expect(anchoredTodo.tokens).toBe(800) // 500 + 300 + expect(anchoredTodo.cost).toBe(0.05) + }) + + it("reopenParentFromDelegation does NOT apply fallback when childIds doesn't include the child", async () => { + const provider = { + contextProxy: { globalStorageUri: { fsPath: "/storage" } }, + getTaskWithId: vi.fn().mockImplementation((taskId: string) => { + if (taskId === "parent-no-relation") { + return Promise.resolve({ + historyItem: { + id: "parent-no-relation", + status: "delegated", + awaitingChildId: "some-other-child", + childIds: ["some-other-child"], // Does NOT include child-orphan + ts: 100, + task: "Parent task", + tokensIn: 0, + tokensOut: 0, + totalCost: 0, + }, + }) + } + return Promise.resolve({ + historyItem: { + id: "child-orphan", + tokensIn: 100, + tokensOut: 50, + totalCost: 0.01, + ts: 200, + task: "Orphan child", + }, + }) + }), + emit: vi.fn(), + getCurrentTask: vi.fn(() => ({ taskId: "child-orphan" })), + removeClineFromStack: vi.fn().mockResolvedValue(undefined), + createTaskWithHistoryItem: vi.fn().mockResolvedValue({ + taskId: "parent-no-relation", + resumeAfterDelegation: vi.fn().mockResolvedValue(undefined), + }), + updateTaskHistory: vi.fn().mockResolvedValue([]), + } as unknown as ClineProvider + + const parentMessagesWithTodos = [ + { + type: "say", + say: "system_update_todos", + text: JSON.stringify({ + tool: "updateTodoList", + todos: [{ id: "todo-1", content: "Some task", status: "completed" }], + }), + ts: 50, + }, + ] + + vi.mocked(readTaskMessages).mockResolvedValue(parentMessagesWithTodos as any) + vi.mocked(readApiMessages).mockResolvedValue([]) + + await (ClineProvider.prototype as any).reopenParentFromDelegation.call(provider, { + parentTaskId: "parent-no-relation", + childTaskId: "child-orphan", + completionResultSummary: "Orphan child completed", + }) + + // Verify saveTaskMessages was called + expect(saveTaskMessages).toHaveBeenCalled() + const savedCall = vi.mocked(saveTaskMessages).mock.calls[0][0] + + // Find todo edit messages (if any were added beyond the original) + const todoEditMessages = savedCall.messages.filter( + (m: any) => m.type === "say" && m.say === "system_update_todos", + ) + + // Should only have the original todo edit, no write-back because child isn't in childIds + // The fallback should NOT be triggered for an unrelated child + if (todoEditMessages.length > 1) { + const lastTodoEdit = todoEditMessages[todoEditMessages.length - 1] + const parsedTodos = JSON.parse(lastTodoEdit.text as string) + // If a write-back happened, it should NOT have linked to child-orphan + const orphanLinked = parsedTodos.todos.find((t: any) => t.subtaskId === "child-orphan") + expect(orphanLinked).toBeUndefined() + } + }) + + it("subtask completion awaits late usage persistence before delegating (parent sees final cost)", async () => { + const parentTaskId = "p-late-cost" + const childTaskId = "c-late-cost" + + // Seed parent messages with a todo linked to the child + const todos = [{ id: "t1", content: "do subtask", status: "in_progress", subtaskId: childTaskId }] + const seededParentMessages = [ + { type: "say", say: "system_update_todos", text: JSON.stringify({ tool: "updateTodoList", todos }), ts: 1 }, + ] as any + vi.mocked(readTaskMessages).mockResolvedValue(seededParentMessages) + vi.mocked(readApiMessages).mockResolvedValue([] as any) + + // Parent history confirms relationship + const parentHistory = { + id: parentTaskId, + status: "delegated", + awaitingChildId: childTaskId, + childIds: [childTaskId], + ts: 1, + task: "Parent", + tokensIn: 0, + tokensOut: 0, + totalCost: 0, + } as any + + // Child history initially stale cost + let childHistory = { + id: childTaskId, + status: "active", + tokensIn: 10, + tokensOut: 5, + totalCost: 0, + ts: 2, + task: "Child", + } as any + + const reopenSpy = vi.fn().mockResolvedValue(undefined) + const provider: any = { + contextProxy: { globalStorageUri: { fsPath: "/storage" } }, + getTaskWithId: vi.fn(async (id: string) => { + if (id === parentTaskId) return { historyItem: parentHistory } + if (id === childTaskId) return { historyItem: childHistory } + throw new Error("unknown") + }), + reopenParentFromDelegation: reopenSpy, + } + + const childTask: any = { + parentTaskId, + taskId: childTaskId, + providerRef: { deref: () => provider }, + didToolFailInCurrentTurn: false, + todoList: undefined, + consecutiveMistakeCount: 0, + recordToolError: vi.fn(), + say: vi.fn(), + emitFinalTokenUsageUpdate: vi.fn(), + getTokenUsage: () => ({}) as any, + toolUsage: {}, + emit: vi.fn(), + ask: vi.fn().mockResolvedValue({ response: "yesButtonClicked" }), + waitForPendingUsageCollection: vi.fn(async () => { + childHistory = { ...childHistory, totalCost: 1.23 } + }), + } + + const { attemptCompletionTool } = await import("../core/tools/AttemptCompletionTool") + const askFinishSubTaskApproval = vi.fn().mockResolvedValue(true) + const handleError = vi.fn((_context: string, error: Error) => { + // Fail loudly if AttemptCompletionTool hits its catch block + throw error + }) + await attemptCompletionTool.execute({ result: "done" }, childTask, { + askApproval: vi.fn(), + handleError, + pushToolResult: vi.fn(), + removeClosingTag: vi.fn((_: any, s: any) => s), + askFinishSubTaskApproval, + toolDescription: vi.fn(), + toolProtocol: "native", + } as any) + + // Ensure we actually awaited and hit the delegation decision point + expect(childTask.waitForPendingUsageCollection).toHaveBeenCalled() + expect(provider.getTaskWithId).toHaveBeenCalledWith(childTaskId) + expect(askFinishSubTaskApproval).toHaveBeenCalled() + expect(reopenSpy).toHaveBeenCalledOnce() + + // Critical ordering: waitForPendingUsageCollection must run before delegation. + const waitCall = childTask.waitForPendingUsageCollection.mock.invocationCallOrder[0] + const reopenCall = reopenSpy.mock.invocationCallOrder[0] + expect(waitCall).toBeLessThan(reopenCall) + + // Parent roll-up reads the child's persisted history; this ensures cost was finalized + // before delegation begins (the bug fix). + expect(childHistory.totalCost).toBe(1.23) + expect(childHistory.tokensIn + childHistory.tokensOut).toBe(15) + }) }) diff --git a/src/__tests__/new-task-delegation.spec.ts b/src/__tests__/new-task-delegation.spec.ts index b6f6d4d36cd..85b107fdac9 100644 --- a/src/__tests__/new-task-delegation.spec.ts +++ b/src/__tests__/new-task-delegation.spec.ts @@ -42,3 +42,79 @@ describe("Task.startSubtask() metadata-driven delegation", () => { expect(provider.createTask).not.toHaveBeenCalled() }) }) + +describe("Deterministic todo anchor selection for subtaskId linking", () => { + // Helper to simulate the anchor selection algorithm from delegateParentAndOpenChild + function selectDeterministicAnchor( + todos: Array<{ id: string; content: string; status: string; subtaskId?: string }>, + ): { id: string; content: string; status: string; subtaskId?: string } | undefined { + const inProgress = todos.filter((t) => t?.status === "in_progress") + const pending = todos.filter((t) => t?.status === "pending") + const completed = todos.filter((t) => t?.status === "completed") + + if (inProgress.length > 0) { + return inProgress[0] + } else if (pending.length > 0) { + return pending[0] + } else if (completed.length > 0) { + return completed[completed.length - 1] // Last completed + } + return undefined + } + + it("selects first in_progress todo when available", () => { + const todos = [ + { id: "1", content: "Task A", status: "completed" }, + { id: "2", content: "Task B", status: "in_progress" }, + { id: "3", content: "Task C", status: "pending" }, + ] + + const chosen = selectDeterministicAnchor(todos) + expect(chosen?.id).toBe("2") + expect(chosen?.status).toBe("in_progress") + }) + + it("selects first pending todo when no in_progress", () => { + const todos = [ + { id: "1", content: "Task A", status: "completed" }, + { id: "2", content: "Task B", status: "pending" }, + { id: "3", content: "Task C", status: "pending" }, + ] + + const chosen = selectDeterministicAnchor(todos) + expect(chosen?.id).toBe("2") + expect(chosen?.status).toBe("pending") + }) + + it("selects LAST completed todo when all todos are completed", () => { + const todos = [ + { id: "1", content: "Task A", status: "completed" }, + { id: "2", content: "Task B", status: "completed" }, + { id: "3", content: "Task C", status: "completed" }, + ] + + const chosen = selectDeterministicAnchor(todos) + // Should pick the LAST completed (closest to delegation moment) + expect(chosen?.id).toBe("3") + expect(chosen?.content).toBe("Task C") + }) + + it("returns undefined when no todos exist (triggers synthetic anchor creation)", () => { + const todos: Array<{ id: string; content: string; status: string }> = [] + const chosen = selectDeterministicAnchor(todos) + expect(chosen).toBeUndefined() + }) + + it("handles mixed statuses deterministically", () => { + const todos = [ + { id: "1", content: "Done early", status: "completed" }, + { id: "2", content: "In progress", status: "in_progress" }, + { id: "3", content: "Done late", status: "completed" }, + { id: "4", content: "Still pending", status: "pending" }, + ] + + // Should prefer in_progress over everything + const chosen = selectDeterministicAnchor(todos) + expect(chosen?.id).toBe("2") + }) +}) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index af7aed86d58..636c909f052 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -384,6 +384,15 @@ export class Task extends EventEmitter implements TaskLike { presentAssistantMessageHasPendingUpdates = false userMessageContent: (Anthropic.TextBlockParam | Anthropic.ImageBlockParam | Anthropic.ToolResultBlockParam)[] = [] userMessageContentReady = false + /** + * When an LLM stream ends, some providers may emit usage/cost information in trailing chunks. + * We drain the iterator in the background to capture those, which can complete *after* a + * subtask calls attempt_completion. + * + * This promise tracks the currently-running background usage collection so callers (notably + * delegation/roll-up logic) can await final persisted cost/tokens before reading history. + */ + private pendingUsageCollectionPromise?: Promise /** * Push a tool_result block to userMessageContent, preventing duplicates. @@ -1224,6 +1233,23 @@ export class Task extends EventEmitter implements TaskLike { } } + /** + * Best-effort wait for any in-flight background usage collection to finish. + * + * This is critical when completing subtasks: the parent roll-up reads cost/tokens + * from the child's persisted history item, which is updated by `saveClineMessages()`. + */ + public async waitForPendingUsageCollection(timeoutMs: number = DEFAULT_USAGE_COLLECTION_TIMEOUT_MS): Promise { + const pending = this.pendingUsageCollectionPromise + if (!pending) return + + try { + await Promise.race([pending, delay(timeoutMs)]) + } catch { + // Non-fatal: background usage collection already logs errors. + } + } + private findMessageByTimestamp(ts: number): ClineMessage | undefined { for (let i = this.clineMessages.length - 1; i >= 0; i--) { if (this.clineMessages[i].ts === ts) { @@ -3256,10 +3282,24 @@ export class Task extends EventEmitter implements TaskLike { } } - // Start the background task and handle any errors - drainStreamInBackgroundToFindAllUsage(lastApiReqIndex).catch((error) => { - console.error("Background usage collection failed:", error) - }) + // Start the background task and handle any errors. + // IMPORTANT: keep a reference so completion/delegation can await final persisted usage/cost. + this.pendingUsageCollectionPromise = drainStreamInBackgroundToFindAllUsage(lastApiReqIndex) + .catch((error) => { + const err = error instanceof Error ? error : new Error("Background usage collection failed") + TelemetryService.instance.captureException(err, { + location: "Task.pendingUsageCollectionPromise", + taskId: this.taskId, + instanceId: this.instanceId, + lastApiReqIndex, + originalError: error instanceof Error ? undefined : serializeError(error), + }) + }) + .finally(() => { + if (this.pendingUsageCollectionPromise) { + this.pendingUsageCollectionPromise = undefined + } + }) } catch (error) { // Abandoned happens when extension is no longer waiting for the // Cline instance to finish aborting (error is thrown here when diff --git a/src/core/tools/AttemptCompletionTool.ts b/src/core/tools/AttemptCompletionTool.ts index 039036f829c..cb3f8df77b1 100644 --- a/src/core/tools/AttemptCompletionTool.ts +++ b/src/core/tools/AttemptCompletionTool.ts @@ -91,6 +91,9 @@ export class AttemptCompletionTool extends BaseTool<"attempt_completion"> { // This ensures the most recent stats are captured regardless of throttle timer // and properly updates the snapshot to prevent redundant emissions task.emitFinalTokenUsageUpdate() + // Ensure any trailing usage/cost emitted after stream completion is persisted + // before delegation/roll-up reads the child's history item. + await task.waitForPendingUsageCollection() TelemetryService.instance.captureTaskCompleted(task.taskId) task.emit(RooCodeEventName.TaskCompleted, task.taskId, task.getTokenUsage(), task.toolUsage) diff --git a/src/core/tools/UpdateTodoListTool.ts b/src/core/tools/UpdateTodoListTool.ts index f8b3653b9a3..5e3a47afc90 100644 --- a/src/core/tools/UpdateTodoListTool.ts +++ b/src/core/tools/UpdateTodoListTool.ts @@ -2,7 +2,6 @@ import { Task } from "../task/Task" import { formatResponse } from "../prompts/responses" import { BaseTool, ToolCallbacks } from "./BaseTool" import type { ToolUse } from "../../shared/tools" -import cloneDeep from "clone-deep" import crypto from "crypto" import { TodoItem, TodoStatus, todoStatusSchema } from "@roo-code/types" import { getLatestTodo } from "../../shared/todo" @@ -23,23 +22,50 @@ export class UpdateTodoListTool extends BaseTool<"update_todo_list"> { } async execute(params: UpdateTodoListParams, task: Task, callbacks: ToolCallbacks): Promise { - const { pushToolResult, handleError, askApproval, toolProtocol } = callbacks + const { pushToolResult, handleError, askApproval } = callbacks try { + const previousFromMemory = getTodoListForTask(task) + + const previousFromHistory = getLatestTodo(task.clineMessages) as unknown as TodoItem[] | undefined + + const historyHasMetadata = + Array.isArray(previousFromHistory) && + previousFromHistory.some( + (t) => t?.subtaskId !== undefined || t?.tokens !== undefined || t?.cost !== undefined, + ) + + const previousTodos: TodoItem[] = + (previousFromMemory?.length ?? 0) === 0 + ? (previousFromHistory ?? []) + : (previousFromHistory?.length ?? 0) === 0 + ? (previousFromMemory ?? []) + : historyHasMetadata + ? (previousFromHistory ?? []) + : (previousFromMemory ?? []) + const todosRaw = params.todos let todos: TodoItem[] - try { - todos = parseMarkdownChecklist(todosRaw || "") - } catch { + const jsonParseResult = tryParseTodoItemsJson(todosRaw) + if (jsonParseResult.parsed) { + todos = jsonParseResult.parsed + } else if (jsonParseResult.error) { task.consecutiveMistakeCount++ task.recordToolError("update_todo_list") task.didToolFailInCurrentTurn = true - pushToolResult(formatResponse.toolError("The todos parameter is not valid markdown checklist or JSON")) + pushToolResult(formatResponse.toolError(jsonParseResult.error)) return + } else { + // Backward compatible: fall back to markdown checklist parsing when JSON parsing is not applicable. + todos = parseMarkdownChecklist(todosRaw || "") } - const { valid, error } = validateTodos(todos) + // Preserve metadata (subtaskId/tokens/cost) for todos whose content matches an existing todo. + // Matching is by exact content string; duplicates are matched in order. + const todosWithPreservedMetadata = preserveTodoMetadata(todos, previousTodos) + + const { valid, error } = validateTodos(todosWithPreservedMetadata) if (!valid) { task.consecutiveMistakeCount++ task.recordToolError("update_todo_list") @@ -48,10 +74,13 @@ export class UpdateTodoListTool extends BaseTool<"update_todo_list"> { return } - let normalizedTodos: TodoItem[] = todos.map((t) => ({ + let normalizedTodos: TodoItem[] = todosWithPreservedMetadata.map((t) => ({ id: t.id, content: t.content, status: normalizeStatus(t.status), + subtaskId: t.subtaskId, + tokens: t.tokens, + cost: t.cost, })) const approvalMsg = JSON.stringify({ @@ -59,7 +88,8 @@ export class UpdateTodoListTool extends BaseTool<"update_todo_list"> { todos: normalizedTodos, }) - approvedTodoList = cloneDeep(normalizedTodos) + // TodoItem is a flat object shape; a shallow copy is sufficient here. + approvedTodoList = normalizedTodos.map((t) => ({ ...t })) const didApprove = await askApproval("tool", approvalMsg) if (!didApprove) { pushToolResult("User declined to update the todoList.") @@ -70,6 +100,11 @@ export class UpdateTodoListTool extends BaseTool<"update_todo_list"> { approvedTodoList !== undefined && JSON.stringify(normalizedTodos) !== JSON.stringify(approvedTodoList) if (isTodoListChanged) { normalizedTodos = approvedTodoList ?? [] + + // If the user-edited todo list dropped metadata fields, re-apply metadata preservation against + // the previous list (and keep any explicitly provided metadata in the edited list). + normalizedTodos = preserveTodoMetadata(normalizedTodos, previousTodos) + task.say( "user_edit_todos", JSON.stringify({ @@ -94,6 +129,7 @@ export class UpdateTodoListTool extends BaseTool<"update_todo_list"> { override async handlePartial(task: Task, block: ToolUse<"update_todo_list">): Promise { const todosRaw = block.params.todos + const previousTodos = getTodoListForTask(task) ?? (getLatestTodo(task.clineMessages) as unknown as TodoItem[]) // Parse the markdown checklist to maintain consistent format with execute() let todos: TodoItem[] @@ -104,6 +140,9 @@ export class UpdateTodoListTool extends BaseTool<"update_todo_list"> { todos = [] } + // Avoid log spam: partial updates can stream frequently. + todos = preserveTodoMetadata(todos, previousTodos) + const approvalMsg = JSON.stringify({ tool: "updateTodoList", todos: todos, @@ -181,12 +220,224 @@ function normalizeStatus(status: string | undefined): TodoStatus { return "pending" } +/** + * Preserve metadata (subtaskId, tokens, cost) from previous todos onto next todos. + * + * Matching strategy (in priority order): + * 1. **Subtask ID match**: If the next todo has a `subtaskId`, match against previous todos with + * the same `subtaskId`. This is the most stable identifier when content (and derived IDs) + * changes. + * 2. **ID match**: If both todos have an `id` field and they match exactly, preserve metadata. + * This handles the common case where ID is stable across updates. + * 3. **Content match with position awareness**: For todos without matching subtask IDs or IDs, + * fall back to content-based matching. Duplicates are matched in order (first unmatched + * previous with same content gets matched to first unmatched next with same content). + * + * This approach ensures metadata survives status/content changes (which can alter the derived ID) + * and handles duplicates deterministically. + */ +function preserveTodoMetadata(nextTodos: TodoItem[], previousTodos: TodoItem[]): TodoItem[] { + const safePrevious = previousTodos ?? [] + const safeNext = nextTodos ?? [] + + // Track which previous todos have been used (by their index) to avoid double-matching + const usedPreviousIndices = new Set() + + // Build lookup maps for matching strategies. + // - Subtask ID: may have duplicates; match in order for determinism. + // - ID: should be unique; store first occurrence. + // - Content: may have duplicates; match in order for determinism. + const previousBySubtaskId = new Map>() + const previousById = new Map() + const previousByContent = new Map>() + for (let i = 0; i < safePrevious.length; i++) { + const prev = safePrevious[i] + if (!prev) continue + + if (typeof prev.subtaskId === "string") { + const list = previousBySubtaskId.get(prev.subtaskId) + if (list) list.push({ todo: prev, index: i }) + else previousBySubtaskId.set(prev.subtaskId, [{ todo: prev, index: i }]) + } + + if (typeof prev.id === "string" && !previousById.has(prev.id)) { + previousById.set(prev.id, { todo: prev, index: i }) + } + + if (typeof prev.content === "string") { + const normalizedContent = normalizeTodoContentForId(prev.content) + const list = previousByContent.get(normalizedContent) + if (list) list.push({ todo: prev, index: i }) + else previousByContent.set(normalizedContent, [{ todo: prev, index: i }]) + } + } + + // IMPORTANT: use an explicit fill here (vs a sparse array) so checks like + // [`Array.prototype.some()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/some) + // properly visit every index. This is required for the rename-only index-carryover fallback. + const matchedPreviousIndexByNextIndex: Array = new Array(safeNext.length).fill(undefined) + const result: TodoItem[] = new Array(safeNext.length) + + for (let nextIndex = 0; nextIndex < safeNext.length; nextIndex++) { + const next = safeNext[nextIndex] + if (!next) { + result[nextIndex] = next + continue + } + + let matchedPrev: TodoItem | undefined = undefined + let matchedIndex: number | undefined = undefined + let matchStrategy: "subtaskId" | "id" | "content" | "none" = "none" + + // Strategy 0: Try subtaskId-based matching first (most stable across content/ID changes) + if (typeof next.subtaskId === "string") { + const candidates = previousBySubtaskId.get(next.subtaskId) + if (candidates) { + for (const candidate of candidates) { + if (!usedPreviousIndices.has(candidate.index)) { + matchedPrev = candidate.todo + matchedIndex = candidate.index + matchStrategy = "subtaskId" + break + } + } + } + } + + // Strategy 1: Try ID-based matching first (most reliable) + if (!matchedPrev && next.id && typeof next.id === "string") { + const byId = previousById.get(next.id) + if (byId && !usedPreviousIndices.has(byId.index)) { + matchedPrev = byId.todo + matchedIndex = byId.index + matchStrategy = "id" + } + } + + // Strategy 2: Fall back to content-based matching if ID didn't match + if (!matchedPrev && typeof next.content === "string") { + const normalizedContent = normalizeTodoContentForId(next.content) + const candidates = previousByContent.get(normalizedContent) + if (candidates) { + // Find first unused candidate + for (const candidate of candidates) { + if (!usedPreviousIndices.has(candidate.index)) { + matchedPrev = candidate.todo + matchedIndex = candidate.index + matchStrategy = "content" + break + } + } + } + } + + // Mark as used and apply metadata + if (matchedPrev && matchedIndex !== undefined) { + usedPreviousIndices.add(matchedIndex) + matchedPreviousIndexByNextIndex[nextIndex] = matchedIndex + result[nextIndex] = { + ...next, + subtaskId: next.subtaskId ?? matchedPrev.subtaskId, + tokens: next.tokens ?? matchedPrev.tokens, + cost: next.cost ?? matchedPrev.cost, + } + continue + } + + result[nextIndex] = next + } + + // Short-term patch: deterministic “rename-only by index” metadata carryover. + // + // Applies only when: + // - lengths are identical + // - status sequence matches index-for-index + // - at least one row could not be matched by subtaskId/id/content + // + // For unmatched rows, carry over metadata fields by index. + const hasUnmatchedRows = matchedPreviousIndexByNextIndex.some((idx) => idx === undefined) + const canApplyIndexRenameCarryover = + hasUnmatchedRows && + safePrevious.length === safeNext.length && + todoStatusSequenceMatchesByIndex(safePrevious, safeNext) + + if (canApplyIndexRenameCarryover) { + for (let i = 0; i < safeNext.length; i++) { + if (matchedPreviousIndexByNextIndex[i] !== undefined) continue // already matched by stable strategy + if (usedPreviousIndices.has(i)) continue // avoid double-using a previous row + const prev = safePrevious[i] + const next = result[i] + if (!prev || !next) continue + + result[i] = { + ...next, + subtaskId: next.subtaskId ?? prev.subtaskId, + tokens: next.tokens ?? prev.tokens, + cost: next.cost ?? prev.cost, + } + usedPreviousIndices.add(i) + } + } + + // Fallback: carry forward metadata from unmatched delegated todos + // to new todos that don't have a subtaskId yet. + // + // This addresses the case where delegation replaces the original todo content with a synthetic + // placeholder (e.g. "Delegated to subtask") and an ID like "synthetic-{subtaskId}", but the LLM + // later rewrites the todo content entirely. In that scenario, subtaskId/id/content matching can all + // fail, causing the delegated metadata to be lost. + const unmatchedPreviousDelegatedTodos = safePrevious + .map((prev, index) => ({ prev, index })) + .filter(({ prev, index }) => typeof prev?.subtaskId === "string" && !usedPreviousIndices.has(index)) + + const nextTodoIndicesWithoutSubtaskId = result + .map((todo, index) => ({ todo, index })) + .filter(({ todo }) => todo !== undefined && todo.subtaskId === undefined) + .map(({ index }) => index) + + for (let i = 0; i < unmatchedPreviousDelegatedTodos.length && i < nextTodoIndicesWithoutSubtaskId.length; i++) { + const { prev: orphanedPrev, index: orphanedPrevIndex } = unmatchedPreviousDelegatedTodos[i] + const targetNextIndex = nextTodoIndicesWithoutSubtaskId[i] + const targetNext = result[targetNextIndex] + if (!orphanedPrev || !targetNext) continue + + const updatedTarget: TodoItem = { + ...targetNext, + subtaskId: targetNext.subtaskId ?? orphanedPrev.subtaskId, + tokens: targetNext.tokens ?? orphanedPrev.tokens, + cost: targetNext.cost ?? orphanedPrev.cost, + } + + result[targetNextIndex] = updatedTarget + usedPreviousIndices.add(orphanedPrevIndex) + matchedPreviousIndexByNextIndex[targetNextIndex] = orphanedPrevIndex + } + + return result +} + +function todoStatusSequenceMatchesByIndex(previous: TodoItem[], next: TodoItem[]): boolean { + const safePrevious = previous ?? [] + const safeNext = next ?? [] + if (safePrevious.length !== safeNext.length) return false + for (let i = 0; i < safeNext.length; i++) { + const prevStatus = normalizeStatus(safePrevious[i]?.status) + const nextStatus = normalizeStatus(safeNext[i]?.status) + if (prevStatus !== nextStatus) return false + } + return true +} export function parseMarkdownChecklist(md: string): TodoItem[] { if (typeof md !== "string") return [] const lines = md .split(/\r?\n/) .map((l) => l.trim()) .filter(Boolean) + + // Tracks occurrences of the same normalized todo content so duplicate rows get + // deterministic, stable IDs. + const occurrenceByNormalizedContent = new Map() + const todos: TodoItem[] = [] for (const line of lines) { const match = line.match(/^(?:-\s*)?\[\s*([ xX\-~])\s*\]\s+(.+)$/) @@ -194,19 +445,82 @@ export function parseMarkdownChecklist(md: string): TodoItem[] { let status: TodoStatus = "pending" if (match[1] === "x" || match[1] === "X") status = "completed" else if (match[1] === "-" || match[1] === "~") status = "in_progress" - const id = crypto - .createHash("md5") - .update(match[2] + status) - .digest("hex") + + const content = match[2] + const normalizedContent = normalizeTodoContentForId(content) + const occurrence = (occurrenceByNormalizedContent.get(normalizedContent) ?? 0) + 1 + occurrenceByNormalizedContent.set(normalizedContent, occurrence) + + // ID must be stable across status changes. + // For duplicates (same normalized content), include occurrence index. + const id = crypto.createHash("md5").update(`${normalizedContent}#${occurrence}`).digest("hex") todos.push({ id, - content: match[2], + content, status, }) } return todos } +function tryParseTodoItemsJson(raw: string): { parsed?: TodoItem[]; error?: string } { + if (typeof raw !== "string") return {} + const trimmed = raw.trim() + if (trimmed.length === 0) return {} + + // Fast-path: avoid trying JSON.parse for obvious markdown inputs. + // JSON arrays/objects must start with '[' or '{'. + const firstChar = trimmed[0] + if (firstChar !== "[" && firstChar !== "{") return {} + + let parsed: unknown + try { + parsed = JSON.parse(trimmed) + } catch { + return {} + } + + if (!Array.isArray(parsed)) { + // Only support the new structured format when it is a JSON array of TodoItem-like objects. + return {} + } + + const normalized: TodoItem[] = [] + for (const [i, item] of parsed.entries()) { + if (!item || typeof item !== "object") { + return { error: `Item ${i + 1} is not an object` } + } + + const t = item as Record + const id = t.id + const content = t.content + + if (typeof id !== "string" || id.length === 0) return { error: `Item ${i + 1} is missing id` } + if (typeof content !== "string" || content.length === 0) return { error: `Item ${i + 1} is missing content` } + + const normalizedItem: TodoItem = { + id, + content, + status: normalizeStatus(typeof t.status === "string" ? t.status : undefined), + ...(typeof t.subtaskId === "string" ? { subtaskId: t.subtaskId } : {}), + ...(typeof t.tokens === "number" ? { tokens: t.tokens } : {}), + ...(typeof t.cost === "number" ? { cost: t.cost } : {}), + } + + normalized.push(normalizedItem) + } + + const { valid, error } = validateTodos(normalized) + if (!valid) return { error: error || "todos JSON validation failed" } + + return { parsed: normalized } +} + +function normalizeTodoContentForId(content: string): string { + // Normalize whitespace so trivial formatting changes don't churn IDs. + return content.trim().replace(/\s+/g, " ") +} + export function setPendingTodoList(todos: TodoItem[]) { approvedTodoList = todos } diff --git a/src/core/tools/__tests__/updateTodoListTool.spec.ts b/src/core/tools/__tests__/updateTodoListTool.spec.ts index ebe0500d665..766030bc020 100644 --- a/src/core/tools/__tests__/updateTodoListTool.spec.ts +++ b/src/core/tools/__tests__/updateTodoListTool.spec.ts @@ -1,5 +1,5 @@ import { describe, it, expect, beforeEach, vi } from "vitest" -import { parseMarkdownChecklist } from "../UpdateTodoListTool" +import { parseMarkdownChecklist, UpdateTodoListTool, setPendingTodoList } from "../UpdateTodoListTool" import { TodoItem } from "@roo-code/types" describe("parseMarkdownChecklist", () => { @@ -206,7 +206,7 @@ Just some text }) describe("ID generation", () => { - it("should generate consistent IDs for the same content and status", () => { + it("should generate consistent IDs for the same content", () => { const md1 = `[ ] Task 1 [x] Task 2` const md2 = `[ ] Task 1 @@ -225,11 +225,19 @@ Just some text expect(result[0].id).not.toBe(result[1].id) }) - it("should generate different IDs for same content but different status", () => { - const md = `[ ] Task 1 -[x] Task 1` - const result = parseMarkdownChecklist(md) - expect(result[0].id).not.toBe(result[1].id) + it("should generate the same ID for the same content even when status changes", () => { + const pending = parseMarkdownChecklist(`[ ] Task 1`) + const completed = parseMarkdownChecklist(`[x] Task 1`) + expect(pending[0].id).toBe(completed[0].id) + }) + + it("should keep duplicate IDs stable by occurrence even when status changes", () => { + const pending = parseMarkdownChecklist(`[ ] Task 1\n[ ] Task 1`) + const completed = parseMarkdownChecklist(`[x] Task 1\n[x] Task 1`) + expect(pending[0].id).toBe(completed[0].id) + expect(pending[1].id).toBe(completed[1].id) + // Within a single parse, duplicates must not share IDs. + expect(pending[0].id).not.toBe(pending[1].id) }) it("should generate same IDs regardless of dash prefix", () => { @@ -239,5 +247,631 @@ Just some text const result2 = parseMarkdownChecklist(md2) expect(result1[0].id).toBe(result2[0].id) }) + + it("should generate the same IDs for the same content even when whitespace differs", () => { + const md1 = `[ ] Task 1` + const md2 = `[ ] Task 1` + const result1 = parseMarkdownChecklist(md1) + const result2 = parseMarkdownChecklist(md2) + expect(result1[0].id).toBe(result2[0].id) + }) + }) +}) + +describe("UpdateTodoListTool.execute", () => { + beforeEach(() => { + setPendingTodoList([]) + }) + + it("should preserve per-row metadata (subtaskId/tokens/cost) when only statuses change (bulk markdown rewrite)", async () => { + /** + * Regression test: a bulk markdown rewrite often changes the derived todo `id` + * (since [`parseMarkdownChecklist()`](../UpdateTodoListTool.ts:337) hashes + * `content + status`). When only statuses change, we must still preserve the + * existing per-row metadata. This is especially important for duplicates, + * where unstable IDs and/or duplicate IDs can cause metadata to be dropped + * or misapplied. + */ + const initialMd = "[ ] Task A\n[ ] Task B\n[ ] Task A\n[ ] Task C" + const updatedMd = "[x] Task A\n[x] Task B\n[x] Task A\n[ ] Task C" // content identical, only statuses change + + const previousFromMemory: TodoItem[] = parseMarkdownChecklist(initialMd).map((t, idx) => ({ + ...t, + subtaskId: `subtask-${idx + 1}`, + tokens: 1000 + idx, + cost: 0.01 * (idx + 1), + })) + + const task = { + todoList: previousFromMemory, + clineMessages: [], + consecutiveMistakeCount: 0, + recordToolError: vi.fn(), + didToolFailInCurrentTurn: false, + say: vi.fn(), + } as any + + const tool = new UpdateTodoListTool() + await tool.execute({ todos: updatedMd }, task, { + pushToolResult: vi.fn(), + handleError: vi.fn(), + askApproval: vi.fn().mockResolvedValue(true), + removeClosingTag: vi.fn(), + toolProtocol: "xml", + }) + + expect(task.todoList).toHaveLength(4) + + // Preserve per-row metadata (including duplicates) by order. + expect(task.todoList[0]).toEqual( + expect.objectContaining({ + content: "Task A", + status: "completed", + subtaskId: "subtask-1", + tokens: 1000, + cost: 0.01, + }), + ) + + expect(task.todoList[1]).toEqual( + expect.objectContaining({ + content: "Task B", + status: "completed", + subtaskId: "subtask-2", + tokens: 1001, + cost: 0.02, + }), + ) + + expect(task.todoList[2]).toEqual( + expect.objectContaining({ + content: "Task A", + status: "completed", + subtaskId: "subtask-3", + tokens: 1002, + cost: 0.03, + }), + ) + + expect(task.todoList[3]).toEqual( + expect.objectContaining({ + content: "Task C", + status: "pending", + subtaskId: "subtask-4", + tokens: 1003, + cost: 0.04, + }), + ) + }) + + it("should preserve subtaskId/metrics when items are renamed but status sequence and length are unchanged (markdown)", async () => { + const initialMd = "[ ] Old A\n[x] Old B\n[-] Old C" + const updatedMd = "[ ] New A\n[x] New B\n[-] New C" // same length + same status sequence, only content changed + + const previousFromMemory: TodoItem[] = parseMarkdownChecklist(initialMd).map((t, idx) => ({ + ...t, + subtaskId: `subtask-${idx + 1}`, + tokens: 100 + idx, + cost: 0.01 * (idx + 1), + })) + + const task = { + todoList: previousFromMemory, + clineMessages: [], + consecutiveMistakeCount: 0, + recordToolError: vi.fn(), + didToolFailInCurrentTurn: false, + say: vi.fn(), + } as any + + const tool = new UpdateTodoListTool() + await tool.execute({ todos: updatedMd }, task, { + pushToolResult: vi.fn(), + handleError: vi.fn(), + askApproval: vi.fn().mockResolvedValue(true), + removeClosingTag: vi.fn(), + toolProtocol: "xml", + }) + + expect(task.todoList).toHaveLength(3) + + expect(task.todoList[0]).toEqual( + expect.objectContaining({ + content: "New A", + status: "pending", + subtaskId: "subtask-1", + tokens: 100, + cost: 0.01, + }), + ) + expect(task.todoList[1]).toEqual( + expect.objectContaining({ + content: "New B", + status: "completed", + subtaskId: "subtask-2", + tokens: 101, + cost: 0.02, + }), + ) + expect(task.todoList[2]).toEqual( + expect.objectContaining({ + content: "New C", + status: "in_progress", + subtaskId: "subtask-3", + tokens: 102, + cost: 0.03, + }), + ) + }) + + it("should accept JSON TodoItem[] payload and preserve ids/subtask links across renames", async () => { + const previousFromMemory: TodoItem[] = [ + { + id: "id-1", + content: "Alpha", + status: "pending", + subtaskId: "subtask-1", + tokens: 111, + cost: 0.11, + }, + { + id: "id-2", + content: "Beta", + status: "completed", + subtaskId: "subtask-2", + tokens: 222, + cost: 0.22, + }, + ] + + // Reorder + rename while keeping id/subtaskId stable; omit metrics to verify preservation. + const jsonPayload: TodoItem[] = [ + { id: "id-2", content: "Beta renamed", status: "completed", subtaskId: "subtask-2" }, + { id: "id-1", content: "Alpha renamed", status: "pending", subtaskId: "subtask-1" }, + ] + + const task = { + todoList: previousFromMemory, + clineMessages: [], + consecutiveMistakeCount: 0, + recordToolError: vi.fn(), + didToolFailInCurrentTurn: false, + say: vi.fn(), + } as any + + const tool = new UpdateTodoListTool() + await tool.execute({ todos: JSON.stringify(jsonPayload) }, task, { + pushToolResult: vi.fn(), + handleError: vi.fn(), + askApproval: vi.fn().mockResolvedValue(true), + removeClosingTag: vi.fn(), + toolProtocol: "xml", + }) + + expect(task.todoList).toHaveLength(2) + // Order should match the JSON payload. + expect(task.todoList.map((t: TodoItem) => t.id)).toEqual(["id-2", "id-1"]) + + expect(task.todoList[0]).toEqual( + expect.objectContaining({ + id: "id-2", + content: "Beta renamed", + status: "completed", + subtaskId: "subtask-2", + tokens: 222, + cost: 0.22, + }), + ) + + expect(task.todoList[1]).toEqual( + expect.objectContaining({ + id: "id-1", + content: "Alpha renamed", + status: "pending", + subtaskId: "subtask-1", + tokens: 111, + cost: 0.11, + }), + ) + }) + + it("should prefer history todos when they contain metadata (subtaskId/tokens/cost)", async () => { + const md = "[ ] Task 1" + + const previousFromMemory = parseMarkdownChecklist(md) + const previousFromHistory: TodoItem[] = previousFromMemory.map((t) => ({ + ...t, + subtaskId: "subtask-1", + tokens: 123, + cost: 0.01, + })) + + const task = { + todoList: previousFromMemory, + clineMessages: [ + { + type: "ask", + ask: "tool", + text: JSON.stringify({ tool: "updateTodoList", todos: previousFromHistory }), + }, + ], + consecutiveMistakeCount: 0, + recordToolError: vi.fn(), + didToolFailInCurrentTurn: false, + say: vi.fn(), + } as any + + const tool = new UpdateTodoListTool() + await tool.execute({ todos: md }, task, { + pushToolResult: vi.fn(), + handleError: vi.fn(), + askApproval: vi.fn().mockResolvedValue(true), + removeClosingTag: vi.fn(), + toolProtocol: "xml", + }) + + expect(task.todoList).toHaveLength(1) + expect(task.todoList[0]).toEqual( + expect.objectContaining({ + content: "Task 1", + subtaskId: "subtask-1", + tokens: 123, + cost: 0.01, + }), + ) + }) + + it("should preserve metadata by subtaskId even when content (and derived id) changes", async () => { + // This test simulates the "user edited todo list" flow. The tool re-applies metadata + // after approval; subtaskId should be used as the primary match when content/id changes. + const md = "[ ] Old text" + + const previousFromMemory: TodoItem[] = parseMarkdownChecklist(md).map((t) => ({ + ...t, + subtaskId: "subtask-1", + tokens: 123, + cost: 0.01, + })) + + const task = { + todoList: previousFromMemory, + clineMessages: [], + consecutiveMistakeCount: 0, + recordToolError: vi.fn(), + didToolFailInCurrentTurn: false, + say: vi.fn(), + } as any + + // Simulate user-edited todo list with updated content and a different id, but the same subtaskId. + const userEditedTodos: TodoItem[] = [ + { + id: "new-id", + content: "New text", + status: "completed", + subtaskId: "subtask-1", + // tokens/cost intentionally omitted to verify preservation + }, + ] + + const tool = new UpdateTodoListTool() + await tool.execute({ todos: md }, task, { + pushToolResult: vi.fn(), + handleError: vi.fn(), + askApproval: vi.fn().mockImplementation(async () => { + setPendingTodoList(userEditedTodos) + return true + }), + removeClosingTag: vi.fn(), + toolProtocol: "xml", + }) + + expect(task.todoList).toHaveLength(1) + expect(task.todoList[0]).toEqual( + expect.objectContaining({ + id: "new-id", + content: "New text", + status: "completed", + subtaskId: "subtask-1", + tokens: 123, + cost: 0.01, + }), + ) + }) + + it("should preserve metadata when content changes only by whitespace/formatting (legacy ids)", async () => { + const md = "[x] Task 1\n[ ] Task 2" + + const previousFromMemory: TodoItem[] = [ + { + id: "legacy-1", + content: "Task 1", + status: "pending", + tokens: 111, + cost: 0.11, + }, + { + id: "legacy-2", + content: "Task 2", + status: "pending", + tokens: 222, + cost: 0.22, + }, + ] + + const task = { + todoList: previousFromMemory, + clineMessages: [], + consecutiveMistakeCount: 0, + recordToolError: vi.fn(), + didToolFailInCurrentTurn: false, + say: vi.fn(), + } as any + + const tool = new UpdateTodoListTool() + await tool.execute({ todos: md }, task, { + pushToolResult: vi.fn(), + handleError: vi.fn(), + askApproval: vi.fn().mockResolvedValue(true), + removeClosingTag: vi.fn(), + toolProtocol: "xml", + } as any) + + expect(task.todoList).toHaveLength(2) + + const task1 = task.todoList.find((t: TodoItem) => t.content === "Task 1") + const task2 = task.todoList.find((t: TodoItem) => t.content === "Task 2") + + expect(task1).toEqual( + expect.objectContaining({ + content: "Task 1", + status: "completed", + tokens: 111, + cost: 0.11, + }), + ) + expect(task2).toEqual( + expect.objectContaining({ + content: "Task 2", + status: "pending", + tokens: 222, + cost: 0.22, + }), + ) + }) + + it("should carry forward metadata from unmatched delegated todos when the LLM rewrites content", async () => { + const delegatedSubtaskId = "019bdcf3-b738-7779-ba86-a4838b490b40" + const previousFromMemory: TodoItem[] = [ + { + id: `synthetic-${delegatedSubtaskId}`, + content: "Delegated to subtask", + status: "pending", + subtaskId: delegatedSubtaskId, + tokens: 1234, + cost: 0.12, + }, + ] + + // Simulate the LLM rewriting the todo content entirely (no content/id/subtaskId match). + // Use a different-length list so the rename-by-index fallback does NOT apply. + const updatedMd = "[ ] Delegate joke-telling to Ask mode\n[ ] Another unrelated task" + + const task = { + todoList: previousFromMemory, + clineMessages: [], + consecutiveMistakeCount: 0, + recordToolError: vi.fn(), + didToolFailInCurrentTurn: false, + say: vi.fn(), + } as any + + const tool = new UpdateTodoListTool() + await tool.execute({ todos: updatedMd }, task, { + pushToolResult: vi.fn(), + handleError: vi.fn(), + askApproval: vi.fn().mockResolvedValue(true), + removeClosingTag: vi.fn(), + toolProtocol: "xml", + }) + + expect(task.todoList).toHaveLength(2) + expect(task.todoList[0]).toEqual( + expect.objectContaining({ + content: "Delegate joke-telling to Ask mode", + status: "pending", + subtaskId: delegatedSubtaskId, + tokens: 1234, + cost: 0.12, + }), + ) + + // Ensure the second new todo doesn't incorrectly inherit delegated metadata. + expect(task.todoList[1]).toEqual( + expect.objectContaining({ + content: "Another unrelated task", + status: "pending", + }), + ) + expect(task.todoList[1].subtaskId).toBeUndefined() + expect(task.todoList[1].tokens).toBeUndefined() + expect(task.todoList[1].cost).toBeUndefined() + }) + + it("should carry forward metadata from unmatched delegated todos even when the previous id is non-synthetic (sequential updates)", async () => { + const delegatedSubtaskId = "019bdcf3-b738-7779-ba86-a4838b490b41" + const previousFromMemory: TodoItem[] = [ + { + id: `synthetic-${delegatedSubtaskId}`, + content: "Delegated to subtask", + status: "pending", + subtaskId: delegatedSubtaskId, + tokens: 1234, + cost: 0.12, + }, + { + id: "other-1", + content: "Another unrelated task", + status: "pending", + }, + ] + + const task = { + todoList: previousFromMemory, + clineMessages: [], + consecutiveMistakeCount: 0, + recordToolError: vi.fn(), + didToolFailInCurrentTurn: false, + say: vi.fn(), + } as any + + const tool = new UpdateTodoListTool() + + // Update 1: delegated todo gets rewritten into a new markdown todo (ID becomes derived md5, i.e. non-synthetic) + const updatedMd1 = "[ ] Delegate joke-telling to Ask mode\n[ ] Another unrelated task" + await tool.execute({ todos: updatedMd1 }, task, { + pushToolResult: vi.fn(), + handleError: vi.fn(), + askApproval: vi.fn().mockResolvedValue(true), + removeClosingTag: vi.fn(), + toolProtocol: "xml", + }) + + expect(task.todoList).toHaveLength(2) + expect(task.todoList[0]).toEqual( + expect.objectContaining({ + content: "Delegate joke-telling to Ask mode", + subtaskId: delegatedSubtaskId, + tokens: 1234, + cost: 0.12, + }), + ) + // Ensure the carried-over todo now has a non-synthetic ID (this is the regression scenario). + expect(task.todoList[0].id).not.toMatch(/^synthetic-/) + + // Update 2: LLM rewrites the delegated todo content again, and list length changes so index-carryover won't apply. + // No subtaskId is provided in the markdown. + const updatedMd2 = + "[ ] Delegate joke-telling to Ask mode (updated)\n[ ] Another unrelated task\n[ ] Third task added" + await tool.execute({ todos: updatedMd2 }, task, { + pushToolResult: vi.fn(), + handleError: vi.fn(), + askApproval: vi.fn().mockResolvedValue(true), + removeClosingTag: vi.fn(), + toolProtocol: "xml", + }) + + expect(task.todoList).toHaveLength(3) + expect(task.todoList[0]).toEqual( + expect.objectContaining({ + content: "Delegate joke-telling to Ask mode (updated)", + subtaskId: delegatedSubtaskId, + tokens: 1234, + cost: 0.12, + }), + ) + + // Ensure non-delegated todos do not accidentally inherit delegated metadata. + expect(task.todoList[1].subtaskId).toBeUndefined() + expect(task.todoList[2].subtaskId).toBeUndefined() + }) + + it("should not cross-contaminate metadata when no subtaskId is present", async () => { + const initialMd = "[ ] Task 1\n[ ] Task 2" + const md = "[x] Task 1\n[ ] Task 2" // status changes for Task 1 -> derived id changes + + const previousFromMemory: TodoItem[] = parseMarkdownChecklist(initialMd).map((t) => + t.content === "Task 1" ? { ...t, tokens: 111, cost: 0.11 } : { ...t, tokens: 222, cost: 0.22 }, + ) + + const task = { + todoList: previousFromMemory, + clineMessages: [], + consecutiveMistakeCount: 0, + recordToolError: vi.fn(), + didToolFailInCurrentTurn: false, + say: vi.fn(), + } as any + + const tool = new UpdateTodoListTool() + await tool.execute({ todos: md }, task, { + pushToolResult: vi.fn(), + handleError: vi.fn(), + askApproval: vi.fn().mockResolvedValue(true), + removeClosingTag: vi.fn(), + toolProtocol: "xml", + }) + + expect(task.todoList).toHaveLength(2) + + const task1 = task.todoList.find((t: TodoItem) => t.content === "Task 1") + const task2 = task.todoList.find((t: TodoItem) => t.content === "Task 2") + + expect(task1).toEqual( + expect.objectContaining({ + content: "Task 1", + status: "completed", + tokens: 111, + cost: 0.11, + }), + ) + + expect(task2).toEqual( + expect.objectContaining({ + content: "Task 2", + status: "pending", + tokens: 222, + cost: 0.22, + }), + ) + }) + + it("should not preserve metadata when content changes and there is no subtaskId", async () => { + const initialMd = "[ ] Task 1\n[ ] Task 2" + const md = "[x] Task 1 (updated)\n[ ] Task 2" + + const previousFromMemory: TodoItem[] = parseMarkdownChecklist(initialMd).map((t) => + t.content === "Task 1" ? { ...t, tokens: 111, cost: 0.11 } : { ...t, tokens: 222, cost: 0.22 }, + ) + + const task = { + todoList: previousFromMemory, + clineMessages: [], + consecutiveMistakeCount: 0, + recordToolError: vi.fn(), + didToolFailInCurrentTurn: false, + say: vi.fn(), + } as any + + const tool = new UpdateTodoListTool() + await tool.execute({ todos: md }, task, { + pushToolResult: vi.fn(), + handleError: vi.fn(), + askApproval: vi.fn().mockResolvedValue(true), + removeClosingTag: vi.fn(), + toolProtocol: "xml", + }) + + expect(task.todoList).toHaveLength(2) + + const updated = task.todoList.find((t: TodoItem) => t.content === "Task 1 (updated)") + const task2 = task.todoList.find((t: TodoItem) => t.content === "Task 2") + + expect(updated).toEqual( + expect.objectContaining({ + content: "Task 1 (updated)", + status: "completed", + }), + ) + expect(updated?.tokens).toBeUndefined() + expect(updated?.cost).toBeUndefined() + + expect(task2).toEqual( + expect.objectContaining({ + content: "Task 2", + status: "pending", + tokens: 222, + cost: 0.22, + }), + ) }) }) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 33fa12ca78c..0150b3f061b 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -47,7 +47,12 @@ import { DEFAULT_CHECKPOINT_TIMEOUT_SECONDS, getModelId, } from "@roo-code/types" -import { aggregateTaskCostsRecursive, type AggregatedCosts } from "./aggregateTaskCosts" +import { + aggregateTaskCostsRecursive, + buildSubtaskDetails, + type AggregatedCosts, + type SubtaskDetail, +} from "./aggregateTaskCosts" import { TelemetryService } from "@roo-code/telemetry" import { CloudService, BridgeOrchestrator, getRooCodeApiUrl } from "@roo-code/cloud" @@ -99,6 +104,7 @@ import { webviewMessageHandler } from "./webviewMessageHandler" import type { ClineMessage, TodoItem } from "@roo-code/types" import { readApiMessages, saveApiMessages, saveTaskMessages } from "../task-persistence" import { readTaskMessages } from "../task-persistence/taskMessages" +import { getLatestTodo } from "../../shared/todo" import { getNonce } from "./getNonce" import { getUri } from "./getUri" import { REQUESTY_BASE_URL } from "../../shared/utils/requesty" @@ -517,7 +523,6 @@ export class ClineProvider // Create timeout for automatic cleanup const timeoutId = setTimeout(() => { this.clearPendingEditOperation(operationId) - this.log(`[setPendingEditOperation] Automatically cleared stale pending operation: ${operationId}`) }, ClineProvider.PENDING_OPERATION_TIMEOUT_MS) // Store the operation @@ -526,8 +531,6 @@ export class ClineProvider timeoutId, createdAt: Date.now(), }) - - this.log(`[setPendingEditOperation] Set pending operation: ${operationId}`) } /** @@ -545,7 +548,6 @@ export class ClineProvider if (operation) { clearTimeout(operation.timeoutId) this.pendingOperations.delete(operationId) - this.log(`[clearPendingEditOperation] Cleared pending operation: ${operationId}`) return true } return false @@ -559,7 +561,6 @@ export class ClineProvider clearTimeout(operation.timeoutId) } this.pendingOperations.clear() - this.log(`[clearAllPendingEditOperations] Cleared all pending operations`) } /* @@ -577,22 +578,16 @@ export class ClineProvider } async dispose() { - this.log("Disposing ClineProvider...") - // Clear all tasks from the stack. while (this.clineStack.length > 0) { await this.removeClineFromStack() } - this.log("Cleared all tasks") - // Clear all pending edit operations to prevent memory leaks this.clearAllPendingEditOperations() - this.log("Cleared pending operations") if (this.view && "dispose" in this.view) { this.view.dispose() - this.log("Disposed webview") } this.clearWebviewResources() @@ -618,7 +613,6 @@ export class ClineProvider this.skillsManager = undefined this.marketplaceManager?.cleanup() this.customModesManager?.dispose() - this.log("Disposed all disposables") ClineProvider.activeInstances.delete(this) // Clean up any event listeners attached to this provider @@ -838,10 +832,8 @@ export class ClineProvider webviewView.onDidDispose( async () => { if (inTabMode) { - this.log("Disposing ClineProvider instance for tab view") await this.dispose() } else { - this.log("Clearing webview resources for sidebar view") this.clearWebviewResources() // Reset current workspace manager reference when view is disposed this.codeIndexManager = undefined @@ -1026,16 +1018,8 @@ export class ClineProvider // Perform preparation tasks and set up event listeners await this.performPreparationTasks(task) - - this.log( - `[createTaskWithHistoryItem] rehydrated task ${task.taskId}.${task.instanceId} in-place (flicker-free)`, - ) } else { await this.addClineToStack(task) - - this.log( - `[createTaskWithHistoryItem] ${task.parentTask ? "child" : "parent"} task ${task.taskId}.${task.instanceId} instantiated`, - ) } // Check if there's a pending edit after checkpoint restoration @@ -1044,8 +1028,6 @@ export class ClineProvider if (pendingEdit) { this.clearPendingEditOperation(operationId) // Clear the pending edit - this.log(`[createTaskWithHistoryItem] Processing pending edit after checkpoint restoration`) - // Process the pending edit after a short delay to ensure the task is fully initialized setTimeout(async () => { try { @@ -1712,12 +1694,52 @@ export class ClineProvider }> { const { historyItem } = await this.getTaskWithId(taskId) - const aggregatedCosts = await aggregateTaskCostsRecursive(taskId, async (id: string) => { - const result = await this.getTaskWithId(id) - return result.historyItem - }) + const loadTaskHistoryItemForAggregation = async (id: string): Promise => { + // Root task must still error if missing (enforced by getTaskWithId(taskId) above). + if (id === taskId) { + return historyItem + } + + try { + const result = await this.getTaskWithId(id) + return result.historyItem + } catch (error) { + const errno = error as NodeJS.ErrnoException + const message = error instanceof Error ? error.message : String(error) + + // Child tasks may be missing on disk (e.g. pruned task folder). Treat as absent so rollups still render. + if (errno?.code === "ENOENT" || message === "Task not found") { + // getTaskWithId already performs cleanup for the "Task not found" path. + // For an ENOENT race (folder deleted between exists check and read), ensure state is also cleaned. + if (errno?.code === "ENOENT") { + await this.deleteTaskFromState(id) + } + + this.log( + `[ClineProvider#getTaskWithAggregatedCosts] Skipping missing child task ${id} while aggregating costs for ${taskId}: ${message}`, + ) + return undefined + } + + throw error + } + } + + const aggregatedCosts = await aggregateTaskCostsRecursive(taskId, loadTaskHistoryItemForAggregation) - return { historyItem, aggregatedCosts } + // Build subtask details if there are children + let childDetails: SubtaskDetail[] | undefined + if (aggregatedCosts.childBreakdown && Object.keys(aggregatedCosts.childBreakdown).length > 0) { + childDetails = await buildSubtaskDetails(aggregatedCosts.childBreakdown, loadTaskHistoryItemForAggregation) + } + + return { + historyItem, + aggregatedCosts: { + ...aggregatedCosts, + childDetails, + }, + } } async showTaskWithId(id: string) { @@ -2561,7 +2583,9 @@ export class ClineProvider public log(message: string) { this.outputChannel.appendLine(message) - console.log(message) + // IMPORTANT: never write to stdout/stderr from shared core code. + // The Roo CLI runs a TUI that is corrupted by any console output (e.g., mode selector dropdown). + // VS Code extension logs are available via the OutputChannel. } // getters @@ -2864,10 +2888,6 @@ export class ClineProvider await this.addClineToStack(task) - this.log( - `[createTask] ${task.parentTask ? "child" : "parent"} task ${task.taskId}.${task.instanceId} instantiated`, - ) - return task } @@ -2878,8 +2898,6 @@ export class ClineProvider return } - console.log(`[cancelTask] cancelling task ${task.taskId}.${task.instanceId}`) - const { historyItem, uiMessagesFilePath } = await this.getTaskWithId(task.taskId) // Preserve parent and root task information for history item. @@ -2946,8 +2964,6 @@ export class ClineProvider // This is used when the user cancels a task that is not a subtask. public async clearTask(): Promise { if (this.clineStack.length > 0) { - const task = this.clineStack[this.clineStack.length - 1] - console.log(`[clearTask] clearing task ${task.taskId}.${task.instanceId}`) await this.removeClineFromStack() } } @@ -3177,6 +3193,100 @@ export class ClineProvider initialStatus: "active", }) + // 4.5) Direct todo-subtask linking: set todo.subtaskId = childTaskId at delegation-time + // Persist by appending an updateTodoList message to the parent's message history. + // Uses deterministic anchor selection: in_progress > pending > last completed > synthetic anchor. + try { + const globalStoragePath = this.contextProxy.globalStorageUri.fsPath + const parentMessages = await readTaskMessages({ taskId: parentTaskId, globalStoragePath }) + let todos = (getLatestTodo(parentMessages) as unknown as TodoItem[]) ?? [] + + this.log( + `[TODO-DEBUG] delegateParentAndOpenChild loaded parent todos ${JSON.stringify({ + parentTaskId, + childTaskId: child.taskId, + parentMessagesCount: Array.isArray(parentMessages) ? parentMessages.length : undefined, + todosCount: Array.isArray(todos) ? todos.length : undefined, + todos, + })}`, + ) + + // Ensure todos is a valid array + if (!Array.isArray(todos)) { + todos = [] + } + + // Deterministic selection algorithm: + // 1. First in_progress todo + // 2. Else first pending todo + // 3. Else last completed todo (closest to delegation moment) + // 4. Else create a synthetic anchor todo + let chosen: TodoItem | undefined = undefined + + const inProgress = todos.filter((t) => t?.status === "in_progress") + const pending = todos.filter((t) => t?.status === "pending") + const completed = todos.filter((t) => t?.status === "completed") + + if (inProgress.length > 0) { + chosen = inProgress[0] + } else if (pending.length > 0) { + chosen = pending[0] + } else if (completed.length > 0) { + // Pick the LAST completed todo (closest stable anchor to delegation moment) + chosen = completed[completed.length - 1] + } else { + // No todos exist: append a synthetic anchor todo + const syntheticTodo: TodoItem = { + id: `synthetic-${child.taskId}`, + content: "Delegated to subtask", + status: "completed", + subtaskId: child.taskId, + } + todos.push(syntheticTodo) + chosen = syntheticTodo + } + + // Set the subtaskId on the chosen todo (unless it's the synthetic one we just created) + if (chosen && !chosen.subtaskId) { + chosen.subtaskId = child.taskId + } + + // Always persist the updated todo list + this.log( + `[TODO-DEBUG] delegateParentAndOpenChild persisting system_update_todos ${JSON.stringify({ + parentTaskId, + childTaskId: child.taskId, + chosenTodoId: chosen?.id, + chosenTodoStatus: chosen?.status, + chosenTodoSubtaskId: chosen?.subtaskId, + persistTodosCount: Array.isArray(todos) ? todos.length : undefined, + todos, + })}`, + ) + await saveTaskMessages({ + messages: [ + ...parentMessages, + { + ts: Date.now(), + type: "say", + say: "system_update_todos", + text: JSON.stringify({ + tool: "updateTodoList", + todos, + }), + }, + ], + taskId: parentTaskId, + globalStoragePath, + }) + } catch (error) { + this.log( + `[delegateParentAndOpenChild] Failed to persist delegation-time todo link (non-fatal): ${ + error instanceof Error ? error.message : String(error) + }`, + ) + } + // 5) Persist parent delegation metadata try { const { historyItem } = await this.getTaskWithId(parentTaskId) @@ -3218,6 +3328,19 @@ export class ClineProvider const { parentTaskId, childTaskId, completionResultSummary } = params const globalStoragePath = this.contextProxy.globalStorageUri.fsPath + // 0) Load child task history to capture tokens/cost for write-back. + let childHistoryItem: HistoryItem | undefined + try { + const { historyItem } = await this.getTaskWithId(childTaskId) + childHistoryItem = historyItem + } catch (error) { + this.log( + `[reopenParentFromDelegation] Failed to load child history for ${childTaskId} (non-fatal): ${ + error instanceof Error ? error.message : String(error) + }`, + ) + } + // 1) Load parent from history and current persisted messages const { historyItem } = await this.getTaskWithId(parentTaskId) @@ -3255,6 +3378,100 @@ export class ClineProvider ts, } parentClineMessages.push(subtaskUiMessage) + + // 2.5) Persist provider completion write-back: update parent's todo item with tokens/cost. + // Primary: find todo where t.subtaskId === childTaskId. + // Fallback: if not found BUT this parent/child relationship is valid (from historyItem), + // pick the same deterministic anchor todo and set its subtaskId = childTaskId before writing tokens/cost. + try { + let todos = (getLatestTodo(parentClineMessages) as unknown as TodoItem[]) ?? [] + + this.log( + `[TODO-DEBUG] reopenParentFromDelegation loaded parent todos ${JSON.stringify({ + parentTaskId, + childTaskId, + parentClineMessagesCount: Array.isArray(parentClineMessages) + ? parentClineMessages.length + : undefined, + todosCount: Array.isArray(todos) ? todos.length : undefined, + todos, + })}`, + ) + if (!Array.isArray(todos)) { + todos = [] + } + + if (todos.length > 0) { + // Primary lookup by subtaskId + let linkedTodo = todos.find((t) => t?.subtaskId === childTaskId) + let usedAnchorFallbackLinking = false + + // Fallback: if subtaskId link wasn't found but parent history confirms this child belongs to it, + // use the deterministic anchor selection to establish the link now. + if (!linkedTodo && historyItem.childIds?.includes(childTaskId)) { + const inProgress = todos.filter((t) => t?.status === "in_progress") + const pending = todos.filter((t) => t?.status === "pending") + const completed = todos.filter((t) => t?.status === "completed") + + if (inProgress.length > 0) { + linkedTodo = inProgress[0] + } else if (pending.length > 0) { + linkedTodo = pending[0] + } else if (completed.length > 0) { + // Pick the LAST completed todo (same as delegation-time logic) + linkedTodo = completed[completed.length - 1] + } + + // Set the subtaskId on the fallback anchor if found + if (linkedTodo) { + linkedTodo.subtaskId = childTaskId + usedAnchorFallbackLinking = true + } + } + + if (linkedTodo) { + // Lightweight debug instrumentation (once per reopen) when we had to link via anchor + // instead of finding by subtaskId. + if (usedAnchorFallbackLinking) { + this.log( + `[Roo-Debug] [reopenParentFromDelegation] Provider write-back used anchor fallback (no subtaskId match). parent=${parentTaskId} child=${childTaskId}`, + ) + } + + linkedTodo.tokens = (childHistoryItem?.tokensIn || 0) + (childHistoryItem?.tokensOut || 0) + linkedTodo.cost = childHistoryItem?.totalCost || 0 + + this.log( + `[TODO-DEBUG] reopenParentFromDelegation persisting system_update_todos ${JSON.stringify({ + parentTaskId, + childTaskId, + linkedTodoId: linkedTodo?.id, + usedAnchorFallbackLinking, + persistTodosCount: Array.isArray(todos) ? todos.length : undefined, + todos, + })}`, + ) + + parentClineMessages.push({ + ts: Date.now(), + type: "say", + say: "system_update_todos", + text: JSON.stringify({ + tool: "updateTodoList", + todos, + }), + }) + } + } + } catch (error) { + this.log( + `[reopenParentFromDelegation] Failed to write back todo cost/tokens (non-fatal): ${ + error instanceof Error ? error.message : String(error) + }`, + ) + } + + // Persist injected UI records (subtask_result + optional todo write-back) await saveTaskMessages({ messages: parentClineMessages, taskId: parentTaskId, globalStoragePath }) // Find the tool_use_id from the last assistant message's new_task tool_use @@ -3333,7 +3550,7 @@ export class ClineProvider // 3) Update child metadata to "completed" status try { - const { historyItem: childHistory } = await this.getTaskWithId(childTaskId) + const childHistory = childHistoryItem ?? (await this.getTaskWithId(childTaskId)).historyItem await this.updateTaskHistory({ ...childHistory, status: "completed", diff --git a/src/core/webview/__tests__/ClineProvider.reopenParentFromDelegation.writeback.spec.ts b/src/core/webview/__tests__/ClineProvider.reopenParentFromDelegation.writeback.spec.ts new file mode 100644 index 00000000000..7b64ef73f3f --- /dev/null +++ b/src/core/webview/__tests__/ClineProvider.reopenParentFromDelegation.writeback.spec.ts @@ -0,0 +1,231 @@ +// npx vitest run core/webview/__tests__/ClineProvider.reopenParentFromDelegation.writeback.spec.ts + +import { beforeEach, describe, expect, it, vi } from "vitest" +import type { ClineMessage, HistoryItem, TodoItem } from "@roo-code/types" + +// Mock safe-stable-stringify to avoid runtime error +vi.mock("safe-stable-stringify", () => ({ + default: (obj: any) => JSON.stringify(obj), +})) + +// Mock TelemetryService (provider imports it) +vi.mock("@roo-code/telemetry", () => ({ + TelemetryService: { + instance: { + setProvider: vi.fn(), + captureTaskCreated: vi.fn(), + captureTaskCompleted: vi.fn(), + }, + }, +})) + +// vscode mock for ClineProvider imports +vi.mock("vscode", () => { + const window = { + createTextEditorDecorationType: vi.fn(() => ({ dispose: vi.fn() })), + showErrorMessage: vi.fn(), + onDidChangeActiveTextEditor: vi.fn(() => ({ dispose: vi.fn() })), + } + const workspace = { + getConfiguration: vi.fn(() => ({ + get: vi.fn((_key: string, defaultValue: any) => defaultValue), + update: vi.fn(), + })), + workspaceFolders: [], + } + const env = { + machineId: "test-machine", + uriScheme: "vscode", + appName: "VSCode", + language: "en", + sessionId: "sess", + } + const Uri = { file: (p: string) => ({ fsPath: p, toString: () => p }) } + const commands = { executeCommand: vi.fn() } + const ExtensionMode = { Development: 2 } + const version = "1.0.0-test" + return { window, workspace, env, Uri, commands, ExtensionMode, version } +}) + +// Mock persistence helpers used by provider reopen flow BEFORE importing provider +vi.mock("../../task-persistence/taskMessages", () => ({ + readTaskMessages: vi.fn().mockResolvedValue([]), +})) +vi.mock("../../task-persistence", () => ({ + readApiMessages: vi.fn().mockResolvedValue([]), + saveApiMessages: vi.fn().mockResolvedValue(undefined), + saveTaskMessages: vi.fn().mockResolvedValue(undefined), +})) + +import { ClineProvider } from "../ClineProvider" +import { readTaskMessages } from "../../task-persistence/taskMessages" +import { readApiMessages, saveTaskMessages } from "../../task-persistence" + +/** + * Regression: after a bulk todo rewrite (status changes), delegation completion writeback + * must still update the correct parent todo row by `subtaskId` (not by position/anchor) + * and must not break subtask linkage for other todo rows. + */ +describe("ClineProvider.reopenParentFromDelegation() writeback", () => { + beforeEach(() => { + vi.restoreAllMocks() + }) + + it("updates the correct linked todo after bulk status rewrite while preserving subtaskId", async () => { + const parentTaskId = "parent-1" + const child1TaskId = "child-1" + const child2TaskId = "child-2" + + const initialTodos: TodoItem[] = [ + { id: "t1", content: "Prep", status: "pending" }, + { id: "t2", content: "Subtask 1", status: "pending", subtaskId: child1TaskId }, + { id: "t3", content: "Subtask 2", status: "pending", subtaskId: child2TaskId }, + ] + + // Simulate a bulk rewrite (e.g. update_todo_list) that marks earlier items completed + // while preserving `subtaskId` links, but with regenerated todo IDs. + const bulkRewrittenTodos: TodoItem[] = [ + { id: "t1b", content: "Prep", status: "completed" }, + { id: "t2b", content: "Subtask 1", status: "completed", subtaskId: child1TaskId }, + { id: "t3b", content: "Subtask 2", status: "pending", subtaskId: child2TaskId }, + ] + + const parentClineMessages: ClineMessage[] = [ + { + type: "say", + say: "system_update_todos", + ts: 1, + text: JSON.stringify({ tool: "updateTodoList", todos: initialTodos }), + } as unknown as ClineMessage, + { + type: "say", + say: "system_update_todos", + ts: 2, + text: JSON.stringify({ tool: "updateTodoList", todos: bulkRewrittenTodos }), + } as unknown as ClineMessage, + ] + + vi.mocked(readTaskMessages).mockResolvedValue(parentClineMessages as any) + vi.mocked(readApiMessages).mockResolvedValue([ + { + role: "assistant", + content: [{ type: "tool_use", name: "new_task", id: "tool-use-1" }], + ts: 0, + }, + ] as any) + + const historyIndex: Record = { + [parentTaskId]: { + id: parentTaskId, + ts: 1, + task: "Parent", + status: "delegated", + childIds: [child1TaskId, child2TaskId], + mode: "code", + workspace: "/tmp", + tokensIn: 0, + tokensOut: 0, + totalCost: 0, + } as unknown as HistoryItem, + [child1TaskId]: { + id: child1TaskId, + ts: 2, + task: "Child 1", + status: "active", + mode: "code", + workspace: "/tmp", + tokensIn: 1, + tokensOut: 1, + totalCost: 0.01, + } as unknown as HistoryItem, + [child2TaskId]: { + id: child2TaskId, + ts: 3, + task: "Child 2", + status: "active", + mode: "code", + workspace: "/tmp", + tokensIn: 10, + tokensOut: 5, + totalCost: 0.123, + } as unknown as HistoryItem, + } + + const provider = { + contextProxy: { globalStorageUri: { fsPath: "/tmp" } }, + log: vi.fn(), + getTaskWithId: vi.fn(async (id: string) => { + const historyItem = historyIndex[id] + if (!historyItem) throw new Error(`Task not found: ${id}`) + return { + historyItem, + apiConversationHistory: [], + taskDirPath: "/tmp", + apiConversationHistoryFilePath: "/tmp/api.json", + uiMessagesFilePath: "/tmp/ui.json", + } + }), + updateTaskHistory: vi.fn(async (updated: any) => { + historyIndex[updated.id] = updated + return Object.values(historyIndex) + }), + emit: vi.fn(), + getCurrentTask: vi.fn(() => ({ taskId: child2TaskId })), + removeClineFromStack: vi.fn().mockResolvedValue(undefined), + createTaskWithHistoryItem: vi.fn().mockResolvedValue({ + taskId: parentTaskId, + overwriteClineMessages: vi.fn().mockResolvedValue(undefined), + overwriteApiConversationHistory: vi.fn().mockResolvedValue(undefined), + resumeAfterDelegation: vi.fn().mockResolvedValue(undefined), + }), + } as unknown as ClineProvider + + await (ClineProvider.prototype as any).reopenParentFromDelegation.call(provider, { + parentTaskId, + childTaskId: child2TaskId, + completionResultSummary: "Child 2 complete", + }) + + // Capture the saved messages and extract the writeback todo payload. + expect(saveTaskMessages).toHaveBeenCalledWith(expect.objectContaining({ taskId: parentTaskId })) + const savedMessages = vi.mocked(saveTaskMessages).mock.calls.at(-1)![0].messages as ClineMessage[] + const lastTodoUpdate = [...savedMessages] + .reverse() + .find((m) => m.type === "say" && (m as any).say === "system_update_todos") as any + expect(lastTodoUpdate).toBeTruthy() + + const payload = JSON.parse(lastTodoUpdate.text) as { tool: string; todos: TodoItem[] } + expect(payload.tool).toBe("updateTodoList") + expect(payload.todos).toHaveLength(3) + + const updatedChild2Row = payload.todos.find((t) => t.subtaskId === child2TaskId) + const updatedChild1Row = payload.todos.find((t) => t.subtaskId === child1TaskId) + const updatedPrepRow = payload.todos.find((t) => t.content === "Prep") + + expect(updatedChild2Row).toEqual( + expect.objectContaining({ + id: "t3b", + content: "Subtask 2", + status: "pending", + subtaskId: child2TaskId, + tokens: 15, + cost: 0.123, + }), + ) + + // Ensure other rows were not mutated by this child completion writeback. + expect(updatedChild1Row).toEqual( + expect.objectContaining({ + id: "t2b", + content: "Subtask 1", + status: "completed", + subtaskId: child1TaskId, + }), + ) + expect(updatedChild1Row?.tokens).toBeUndefined() + expect(updatedChild1Row?.cost).toBeUndefined() + + expect(updatedPrepRow).toEqual(expect.objectContaining({ id: "t1b", content: "Prep", status: "completed" })) + expect((updatedPrepRow as any)?.subtaskId).toBeUndefined() + }) +}) diff --git a/src/core/webview/__tests__/aggregateTaskCosts.spec.ts b/src/core/webview/__tests__/aggregateTaskCosts.spec.ts index ffb35f5e48c..2b2d297f7bd 100644 --- a/src/core/webview/__tests__/aggregateTaskCosts.spec.ts +++ b/src/core/webview/__tests__/aggregateTaskCosts.spec.ts @@ -1,6 +1,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest" -import { aggregateTaskCostsRecursive } from "../aggregateTaskCosts.js" +import { aggregateTaskCostsRecursive, buildSubtaskDetails } from "../aggregateTaskCosts.js" import type { HistoryItem } from "@roo-code/types" +import type { AggregatedCosts } from "../aggregateTaskCosts.js" describe("aggregateTaskCostsRecursive", () => { let consoleWarnSpy: ReturnType @@ -198,6 +199,64 @@ describe("aggregateTaskCostsRecursive", () => { expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining("Task nonexistent-child not found")) }) + /** + * Regression test: + * - Provider loader may hit ENOENT for missing child history files. + * - Provider hardening converts ENOENT -> undefined. + * - Aggregator must tolerate undefined children and still return partial results. + */ + it("should tolerate ENOENT from underlying history load and still include costs for existing children", async () => { + const mockHistory: Record = { + root: { + id: "root", + totalCost: 1.0, + childIds: ["child-ok", "child-missing"], + } as unknown as HistoryItem, + "child-ok": { + id: "child-ok", + totalCost: 0.5, + childIds: [], + } as unknown as HistoryItem, + } + + const loadHistory = vi.fn(async (id: string) => { + if (id === "child-missing") { + const err = new Error("ENOENT: no such file or directory") as Error & { code?: string } + err.code = "ENOENT" + throw err + } + + return mockHistory[id] + }) + + // Mimic provider behavior: swallow ENOENT and return undefined. + const getTaskHistory = vi.fn(async (id: string) => { + try { + return await loadHistory(id) + } catch (err) { + if ((err as { code?: string })?.code === "ENOENT") { + return undefined + } + throw err + } + }) + + const result = await aggregateTaskCostsRecursive("root", getTaskHistory) + + // Should still include existing child contributions. + expect(result.ownCost).toBe(1.0) + expect(result.childrenCost).toBe(0.5) + expect(result.totalCost).toBe(1.5) + + const childOk = result.childBreakdown?.["child-ok"] + expect(childOk).toBeDefined() + expect(childOk!.totalCost).toBe(0.5) + + // Missing child should not crash aggregation. + expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining("Task child-missing not found")) + expect(loadHistory).toHaveBeenCalledWith("child-missing") + }) + it("should return zero costs for completely missing task", async () => { const mockHistory: Record = {} @@ -324,3 +383,211 @@ describe("aggregateTaskCostsRecursive", () => { expect(result.totalCost).toBe(2.0) }) }) + +describe("buildSubtaskDetails", () => { + it("should build subtask details from child breakdown", async () => { + const childBreakdown: { [childId: string]: AggregatedCosts } = { + "child-1": { + ownCost: 0.5, + childrenCost: 0, + totalCost: 0.5, + }, + "child-2": { + ownCost: 0.3, + childrenCost: 0.2, + totalCost: 0.5, + }, + } + + const mockHistory: Record = { + "child-1": { + id: "child-1", + task: "First subtask", + tokensIn: 100, + tokensOut: 50, + status: "completed", + } as unknown as HistoryItem, + "child-2": { + id: "child-2", + task: "Second subtask with nested children", + tokensIn: 200, + tokensOut: 100, + status: "active", + } as unknown as HistoryItem, + } + + const getTaskHistory = vi.fn(async (id: string) => mockHistory[id]) + + const result = await buildSubtaskDetails(childBreakdown, getTaskHistory) + + expect(result).toHaveLength(2) + + const child1 = result.find((d) => d.id === "child-1") + expect(child1).toBeDefined() + expect(child1!.name).toBe("First subtask") + expect(child1!.tokens).toBe(150) // 100 + 50 + expect(child1!.cost).toBe(0.5) + expect(child1!.status).toBe("completed") + expect(child1!.hasNestedChildren).toBe(false) + + const child2 = result.find((d) => d.id === "child-2") + expect(child2).toBeDefined() + expect(child2!.name).toBe("Second subtask with nested children") + expect(child2!.tokens).toBe(300) // 200 + 100 + expect(child2!.cost).toBe(0.5) + expect(child2!.status).toBe("active") + expect(child2!.hasNestedChildren).toBe(true) // childrenCost > 0 + }) + + it("should truncate long task names to 50 characters", async () => { + const longTaskName = + "This is a very long task name that exceeds fifty characters and should be truncated with ellipsis" + const childBreakdown: { [childId: string]: AggregatedCosts } = { + "child-1": { + ownCost: 1.0, + childrenCost: 0, + totalCost: 1.0, + }, + } + + const mockHistory: Record = { + "child-1": { + id: "child-1", + task: longTaskName, + tokensIn: 100, + tokensOut: 50, + status: "completed", + } as unknown as HistoryItem, + } + + const getTaskHistory = vi.fn(async (id: string) => mockHistory[id]) + + const result = await buildSubtaskDetails(childBreakdown, getTaskHistory) + + expect(result).toHaveLength(1) + expect(result[0].name).toBe("This is a very long task name that exceeds fift...") + expect(result[0].name.length).toBe(50) + }) + + it("should not truncate task names at or under 50 characters", async () => { + const exactlyFiftyChars = "12345678901234567890123456789012345678901234567890" // exactly 50 chars + const childBreakdown: { [childId: string]: AggregatedCosts } = { + "child-1": { + ownCost: 1.0, + childrenCost: 0, + totalCost: 1.0, + }, + } + + const mockHistory: Record = { + "child-1": { + id: "child-1", + task: exactlyFiftyChars, + tokensIn: 100, + tokensOut: 50, + status: "completed", + } as unknown as HistoryItem, + } + + const getTaskHistory = vi.fn(async (id: string) => mockHistory[id]) + + const result = await buildSubtaskDetails(childBreakdown, getTaskHistory) + + expect(result[0].name).toBe(exactlyFiftyChars) + expect(result[0].name.length).toBe(50) + }) + + it("should skip children with missing history", async () => { + const childBreakdown: { [childId: string]: AggregatedCosts } = { + "child-1": { + ownCost: 0.5, + childrenCost: 0, + totalCost: 0.5, + }, + "missing-child": { + ownCost: 0.3, + childrenCost: 0, + totalCost: 0.3, + }, + } + + const mockHistory: Record = { + "child-1": { + id: "child-1", + task: "Existing subtask", + tokensIn: 100, + tokensOut: 50, + status: "completed", + } as unknown as HistoryItem, + // missing-child has no history + } + + const getTaskHistory = vi.fn(async (id: string) => mockHistory[id]) + + const result = await buildSubtaskDetails(childBreakdown, getTaskHistory) + + expect(result).toHaveLength(1) + expect(result[0].id).toBe("child-1") + }) + + it("should handle empty child breakdown", async () => { + const childBreakdown: { [childId: string]: AggregatedCosts } = {} + + const getTaskHistory = vi.fn(async () => undefined) + + const result = await buildSubtaskDetails(childBreakdown, getTaskHistory) + + expect(result).toHaveLength(0) + }) + + it("should default status to completed when undefined", async () => { + const childBreakdown: { [childId: string]: AggregatedCosts } = { + "child-1": { + ownCost: 0.5, + childrenCost: 0, + totalCost: 0.5, + }, + } + + const mockHistory: Record = { + "child-1": { + id: "child-1", + task: "Subtask without status", + tokensIn: 100, + tokensOut: 50, + // status is undefined + } as unknown as HistoryItem, + } + + const getTaskHistory = vi.fn(async (id: string) => mockHistory[id]) + + const result = await buildSubtaskDetails(childBreakdown, getTaskHistory) + + expect(result[0].status).toBe("completed") + }) + + it("should handle undefined token values", async () => { + const childBreakdown: { [childId: string]: AggregatedCosts } = { + "child-1": { + ownCost: 0.5, + childrenCost: 0, + totalCost: 0.5, + }, + } + + const mockHistory: Record = { + "child-1": { + id: "child-1", + task: "Subtask without tokens", + // tokensIn and tokensOut are undefined + status: "completed", + } as unknown as HistoryItem, + } + + const getTaskHistory = vi.fn(async (id: string) => mockHistory[id]) + + const result = await buildSubtaskDetails(childBreakdown, getTaskHistory) + + expect(result[0].tokens).toBe(0) + }) +}) diff --git a/src/core/webview/aggregateTaskCosts.ts b/src/core/webview/aggregateTaskCosts.ts index 3100b2a65e7..ee8cb14acd8 100644 --- a/src/core/webview/aggregateTaskCosts.ts +++ b/src/core/webview/aggregateTaskCosts.ts @@ -1,5 +1,17 @@ import type { HistoryItem } from "@roo-code/types" +/** + * Detailed information about a subtask for UI display + */ +export interface SubtaskDetail { + id: string // Task ID + name: string // First 50 chars of task description + tokens: number // tokensIn + tokensOut + cost: number // Aggregated total cost + status: "active" | "completed" | "delegated" + hasNestedChildren: boolean // Has its own subtasks +} + export interface AggregatedCosts { ownCost: number // This task's own API costs childrenCost: number // Sum of all direct children costs (recursive) @@ -8,6 +20,7 @@ export interface AggregatedCosts { // Optional detailed breakdown [childId: string]: AggregatedCosts } + childDetails?: SubtaskDetail[] // Detailed subtask info for UI display } /** @@ -26,7 +39,11 @@ export async function aggregateTaskCostsRecursive( // Prevent infinite loops if (visited.has(taskId)) { console.warn(`[aggregateTaskCostsRecursive] Circular reference detected: ${taskId}`) - return { ownCost: 0, childrenCost: 0, totalCost: 0 } + return { + ownCost: 0, + childrenCost: 0, + totalCost: 0, + } } visited.add(taskId) @@ -34,7 +51,11 @@ export async function aggregateTaskCostsRecursive( const history = await getTaskHistory(taskId) if (!history) { console.warn(`[aggregateTaskCostsRecursive] Task ${taskId} not found`) - return { ownCost: 0, childrenCost: 0, totalCost: 0 } + return { + ownCost: 0, + childrenCost: 0, + totalCost: 0, + } } const ownCost = history.totalCost || 0 @@ -63,3 +84,39 @@ export async function aggregateTaskCostsRecursive( return result } + +/** + * Truncate a task name to a maximum length, adding ellipsis if needed + */ +function truncateTaskName(task: string, maxLength: number): string { + if (task.length <= maxLength) return task + return task.substring(0, maxLength - 3) + "..." +} + +/** + * Build subtask details from child breakdown and history items + * for displaying in the UI's expandable subtask list + */ +export async function buildSubtaskDetails( + childBreakdown: { [childId: string]: AggregatedCosts }, + getTaskHistory: (id: string) => Promise, +): Promise { + const details: SubtaskDetail[] = [] + + for (const [childId, costs] of Object.entries(childBreakdown)) { + const history = await getTaskHistory(childId) + + if (history) { + details.push({ + id: childId, + name: truncateTaskName(history.task, 50), + tokens: (history.tokensIn || 0) + (history.tokensOut || 0), + cost: costs.totalCost, + status: history.status || "completed", + hasNestedChildren: costs.childrenCost > 0, + }) + } + } + + return details +} diff --git a/src/shared/__tests__/messageUtils.spec.ts b/src/shared/__tests__/messageUtils.spec.ts new file mode 100644 index 00000000000..f3554fd9d16 --- /dev/null +++ b/src/shared/__tests__/messageUtils.spec.ts @@ -0,0 +1,3 @@ +// This file previously tested getLineStatsFromToolApprovalMessages, +// which has been removed as part of eliminating line change tracking. +// The file is kept as a placeholder for future messageUtils tests. diff --git a/src/shared/__tests__/typeGuards.spec.ts b/src/shared/__tests__/typeGuards.spec.ts new file mode 100644 index 00000000000..45ad67686bd --- /dev/null +++ b/src/shared/__tests__/typeGuards.spec.ts @@ -0,0 +1,30 @@ +// npx vitest run src/shared/__tests__/typeGuards.spec.ts + +import { isFiniteNumber } from "../typeGuards" + +describe("typeGuards", () => { + describe("isFiniteNumber", () => { + it("should return true for finite numbers", () => { + expect(isFiniteNumber(0)).toBe(true) + expect(isFiniteNumber(42)).toBe(true) + expect(isFiniteNumber(-10)).toBe(true) + expect(isFiniteNumber(3.14)).toBe(true) + expect(isFiniteNumber(-0.5)).toBe(true) + }) + + it("should return false for non-finite numbers", () => { + expect(isFiniteNumber(Infinity)).toBe(false) + expect(isFiniteNumber(-Infinity)).toBe(false) + expect(isFiniteNumber(NaN)).toBe(false) + }) + + it("should return false for non-number types", () => { + expect(isFiniteNumber("42")).toBe(false) + expect(isFiniteNumber(null)).toBe(false) + expect(isFiniteNumber(undefined)).toBe(false) + expect(isFiniteNumber(true)).toBe(false) + expect(isFiniteNumber({})).toBe(false) + expect(isFiniteNumber([])).toBe(false) + }) + }) +}) diff --git a/src/shared/messageUtils.ts b/src/shared/messageUtils.ts new file mode 100644 index 00000000000..f284e57405f --- /dev/null +++ b/src/shared/messageUtils.ts @@ -0,0 +1,2 @@ +// This file was emptied as part of removing line change processing from backend +// The getLineStatsFromToolApprovalMessages() function has been removed diff --git a/src/shared/todo.ts b/src/shared/todo.ts index d20539049b0..d2a01225d79 100644 --- a/src/shared/todo.ts +++ b/src/shared/todo.ts @@ -1,25 +1,34 @@ -import { ClineMessage } from "@roo-code/types" +import { ClineMessage, TodoItem } from "@roo-code/types" -export function getLatestTodo(clineMessages: ClineMessage[]) { - const todos = clineMessages - .filter( - (msg) => - (msg.type === "ask" && msg.ask === "tool") || (msg.type === "say" && msg.say === "user_edit_todos"), - ) - .map((msg) => { - try { - return JSON.parse(msg.text ?? "{}") - } catch { - return null - } - }) - .filter((item) => item && item.tool === "updateTodoList" && Array.isArray(item.todos)) - .map((item) => item.todos) - .pop() - - if (todos) { - return todos - } else { +export function getLatestTodo(clineMessages: ClineMessage[]): TodoItem[] { + if (!Array.isArray(clineMessages) || clineMessages.length === 0) { return [] } + + const candidateMessages = clineMessages.filter( + (msg) => + (msg.type === "ask" && msg.ask === "tool") || + (msg.type === "say" && (msg.say === "user_edit_todos" || msg.say === "system_update_todos")), + ) + + let lastTodos: TodoItem[] | undefined + let matchedUpdateTodoListCount = 0 + let parseFailureCount = 0 + + for (const msg of candidateMessages) { + let parsed: any + try { + parsed = JSON.parse(msg.text ?? "{}") + } catch { + parseFailureCount++ + continue + } + + if (parsed && parsed.tool === "updateTodoList" && Array.isArray(parsed.todos)) { + matchedUpdateTodoListCount++ + lastTodos = parsed.todos as TodoItem[] + } + } + + return Array.isArray(lastTodos) ? lastTodos : [] } diff --git a/src/shared/typeGuards.ts b/src/shared/typeGuards.ts new file mode 100644 index 00000000000..a884f66f68a --- /dev/null +++ b/src/shared/typeGuards.ts @@ -0,0 +1,8 @@ +/** + * Type guard to check if a value is a finite number + * @param value - The value to check + * @returns true if the value is a number and is finite + */ +export function isFiniteNumber(value: unknown): value is number { + return typeof value === "number" && Number.isFinite(value) +} diff --git a/webview-ui/src/components/chat/ChatRow.tsx b/webview-ui/src/components/chat/ChatRow.tsx index 24749bb4191..9fbcffcfef5 100644 --- a/webview-ui/src/components/chat/ChatRow.tsx +++ b/webview-ui/src/components/chat/ChatRow.tsx @@ -127,13 +127,19 @@ interface ChatRowContentProps extends Omit {} const ChatRow = memo( (props: ChatRowProps) => { const { isLast, onHeightChange, message } = props + + // Some message types update state but should not be visible in the chat history. + // We still render a row for Virtuoso compatibility, but hide it so it takes up no space. + const shouldHideRow = message.type === "say" && message.say === "system_update_todos" // Store the previous height to compare with the current height // This allows us to detect changes without causing re-renders const prevHeightRef = useRef(0) const [chatrow, { height }] = useSize( -
- +
+ {shouldHideRow ? null : }
, ) @@ -387,10 +393,12 @@ export const ChatRowContent = ({ wordBreak: "break-word", } - const tool = useMemo( - () => (message.ask === "tool" ? safeJsonParse(message.text) : null), - [message.ask, message.text], - ) + const tool = useMemo(() => { + if (message.ask !== "tool") return null + const parsed = safeJsonParse(message.text) + + return parsed + }, [message.ask, message.text]) // Unified diff content (provided by backend when relevant) const unifiedDiff = useMemo(() => { @@ -1418,6 +1426,8 @@ export const ChatRowContent = ({ return case "user_edit_todos": return {}} /> + case "system_update_todos": + return null case "tool" as any: // Handle say tool messages const sayTool = safeJsonParse(message.text) diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index 81f6cbebf66..0786699ca12 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -48,6 +48,7 @@ import { QueuedMessages } from "./QueuedMessages" import DismissibleUpsell from "../common/DismissibleUpsell" import { useCloudUpsell } from "@src/hooks/useCloudUpsell" import { Cloud } from "lucide-react" +import type { SubtaskDetail } from "@src/types/subtasks" export interface ChatViewProps { isHidden: boolean @@ -124,6 +125,10 @@ const ChatViewComponent: React.ForwardRefRenderFunction { + // Intentionally left blank: keep dependencies so todo extraction logic remains reactive. + }, [messages, currentTaskTodos, latestTodos]) + const modifiedMessages = useMemo(() => combineApiRequests(combineCommandSequences(messages.slice(1))), [messages]) // Has to be after api_req_finished are all reduced into api_req_started messages. @@ -174,6 +179,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction >(new Map()) @@ -1490,6 +1496,11 @@ const ChatViewComponent: React.ForwardRefRenderFunction void @@ -61,6 +64,7 @@ const TaskHeader = ({ aggregatedCost, hasSubtasks, costBreakdown, + subtaskDetails, contextTokens, buttonsDisabled, handleCondenseContext, @@ -81,7 +85,7 @@ const TaskHeader = ({ ? (() => { const lastRelevantIndex = findLastIndex( clineMessages, - (m) => !(m.ask === "resume_task" || m.ask === "resume_completed_task"), + (m) => !((m as any)?.ask === "resume_task" || (m as any)?.ask === "resume_completed_task"), ) return lastRelevantIndex !== -1 ? clineMessages[lastRelevantIndex]?.ask === "completion_result" @@ -126,6 +130,57 @@ const TaskHeader = ({ const hasTodos = todos && Array.isArray(todos) && todos.length > 0 + const subtaskCosts = useMemo(() => { + const processedSubtasks = new Set() + const costs: number[] = [] + + const maybeAddCost = (subtaskId: unknown, cost: unknown) => { + if (typeof subtaskId !== "string" || subtaskId.length === 0) return + if (processedSubtasks.has(subtaskId)) return + if (typeof cost !== "number" || !Number.isFinite(cost) || cost <= 0) return + + processedSubtasks.add(subtaskId) + costs.push(cost) + } + + // Primary source of truth: visible todos. + if (Array.isArray(todos)) { + for (const todo of todos) { + maybeAddCost((todo as any)?.subtaskId, (todo as any)?.cost) + } + } + + // Fallback: any remaining subtasks from history-derived details. + if (Array.isArray(subtaskDetails)) { + for (const subtask of subtaskDetails) { + maybeAddCost(subtask.id, subtask.cost) + } + } + + return costs + }, [todos, subtaskDetails]) + + const tooltipCostData = useMemo( + () => + getTaskHeaderCostTooltipData({ + ownCost: totalCost, + aggregatedCost, + hasSubtasksProp: hasSubtasks, + costBreakdownProp: costBreakdown, + subtaskCosts, + labels: { + own: t("common:costs.own"), + subtasks: t("common:costs.subtasks"), + }, + }), + [totalCost, aggregatedCost, hasSubtasks, costBreakdown, subtaskCosts, t], + ) + + const displayTotalCost = tooltipCostData.displayTotalCost + const displayCostBreakdown = tooltipCostData.displayCostBreakdown + const shouldTreatAsHasSubtasks = tooltipCostData.hasSubtasks + const hasAnyCost = tooltipCostData.hasAnyCost + return (
{showLongRunningTaskMessage && !isTaskComplete && ( @@ -254,17 +309,19 @@ const TaskHeader = ({ {formatLargeNumber(contextTokens || 0)} / {formatLargeNumber(contextWindow)} - {!!totalCost && ( + {hasAnyCost && ( + shouldTreatAsHasSubtasks ? ( +
{t("chat:costs.totalWithSubtasks", { - cost: (aggregatedCost ?? totalCost).toFixed(2), + cost: displayTotalCost.toFixed(2), })}
- {costBreakdown &&
{costBreakdown}
} + {displayCostBreakdown && ( +
{displayCostBreakdown}
+ )}
) : (
{t("chat:costs.total", { cost: totalCost.toFixed(2) })}
@@ -272,9 +329,9 @@ const TaskHeader = ({ } side="top" sideOffset={8}> - - ${(aggregatedCost ?? totalCost).toFixed(2)} - {hasSubtasks && ( + + ${displayTotalCost.toFixed(2)} + {shouldTreatAsHasSubtasks && ( * @@ -413,7 +470,7 @@ const TaskHeader = ({ )} - {!!totalCost && ( + {hasAnyCost && ( {t("chat:task.apiCost")} @@ -421,15 +478,17 @@ const TaskHeader = ({
{t("chat:costs.totalWithSubtasks", { - cost: (aggregatedCost ?? totalCost).toFixed(2), + cost: displayTotalCost.toFixed(2), })}
- {costBreakdown && ( -
{costBreakdown}
+ {displayCostBreakdown && ( +
+ {displayCostBreakdown} +
)}
) : ( @@ -441,8 +500,8 @@ const TaskHeader = ({ side="top" sideOffset={8}> - ${(aggregatedCost ?? totalCost).toFixed(2)} - {hasSubtasks && ( + ${displayTotalCost.toFixed(2)} + {shouldTreatAsHasSubtasks && ( @@ -472,7 +531,15 @@ const TaskHeader = ({ )} {/* Todo list - always shown at bottom when todos exist */} - {hasTodos && } + {hasTodos && ( + { + vscode.postMessage({ type: "showTaskWithId", text: subtaskId }) + }} + /> + )}
diff --git a/webview-ui/src/components/chat/TodoChangeDisplay.tsx b/webview-ui/src/components/chat/TodoChangeDisplay.tsx index 3904cd9c9ff..e5f9f86043f 100644 --- a/webview-ui/src/components/chat/TodoChangeDisplay.tsx +++ b/webview-ui/src/components/chat/TodoChangeDisplay.tsx @@ -39,11 +39,13 @@ export function TodoChangeDisplay({ previousTodos, newTodos }: TodoChangeDisplay todosToDisplay = newTodos.filter((newTodo) => { if (newTodo.status === "completed") { const previousTodo = previousTodos.find((p) => p.id === newTodo.id || p.content === newTodo.content) - return !previousTodo || previousTodo.status !== "completed" + const include = !previousTodo || previousTodo.status !== "completed" + return include } if (newTodo.status === "in_progress") { const previousTodo = previousTodos.find((p) => p.id === newTodo.id || p.content === newTodo.content) - return !previousTodo || previousTodo.status !== "in_progress" + const include = !previousTodo || previousTodo.status !== "in_progress" + return include } return false }) diff --git a/webview-ui/src/components/chat/TodoListDisplay.tsx b/webview-ui/src/components/chat/TodoListDisplay.tsx index f2dbbc4d80d..1dd5288f611 100644 --- a/webview-ui/src/components/chat/TodoListDisplay.tsx +++ b/webview-ui/src/components/chat/TodoListDisplay.tsx @@ -3,8 +3,26 @@ import { t } from "i18next" import { ArrowRight, Check, ListChecks, SquareDashed } from "lucide-react" import { useState, useRef, useMemo, useEffect } from "react" +import { formatLargeNumber } from "@src/utils/format" + +import type { SubtaskDetail } from "@src/types/subtasks" + type TodoStatus = "completed" | "in_progress" | "pending" +interface TodoItem { + // Legacy fields + id?: string + content: string + status?: TodoStatus | string | null + + // Direct-linking/cost fields (optional for backward compatibility) + subtaskId?: string + tokens?: number + cost?: number + added?: number + removed?: number +} + function getTodoIcon(status: TodoStatus | null) { switch (status) { case "completed": @@ -16,21 +34,27 @@ function getTodoIcon(status: TodoStatus | null) { } } -export function TodoListDisplay({ todos }: { todos: any[] }) { +export interface TodoListDisplayProps { + todos: TodoItem[] + subtaskDetails?: SubtaskDetail[] + onSubtaskClick?: (subtaskId: string) => void +} + +export function TodoListDisplay({ todos, subtaskDetails, onSubtaskClick }: TodoListDisplayProps) { const [isCollapsed, setIsCollapsed] = useState(true) const ulRef = useRef(null) const itemRefs = useRef<(HTMLLIElement | null)[]>([]) const scrollIndex = useMemo(() => { - const inProgressIdx = todos.findIndex((todo: any) => todo.status === "in_progress") + const inProgressIdx = todos.findIndex((todo) => todo.status === "in_progress") if (inProgressIdx !== -1) return inProgressIdx - return todos.findIndex((todo: any) => todo.status !== "completed") + return todos.findIndex((todo) => todo.status !== "completed") }, [todos]) // Find the most important todo to display when collapsed const mostImportantTodo = useMemo(() => { - const inProgress = todos.find((todo: any) => todo.status === "in_progress") + const inProgress = todos.find((todo) => todo.status === "in_progress") if (inProgress) return inProgress - return todos.find((todo: any) => todo.status !== "completed") + return todos.find((todo) => todo.status !== "completed") }, [todos]) useEffect(() => { if (isCollapsed) return @@ -49,7 +73,7 @@ export function TodoListDisplay({ todos }: { todos: any[] }) { if (!Array.isArray(todos) || todos.length === 0) return null const totalCount = todos.length - const completedCount = todos.filter((todo: any) => todo.status === "completed").length + const completedCount = todos.filter((todo) => todo.status === "completed").length const allCompleted = completedCount === totalCount && totalCount > 0 @@ -62,7 +86,7 @@ export function TodoListDisplay({ todos }: { todos: any[] }) { ? "text-vscode-charts-yellow" : "text-vscode-foreground", )} - onClick={() => setIsCollapsed((v) => !v)}> + onClick={() => setIsCollapsed((v: boolean) => !v)}> {isCollapsed @@ -80,19 +104,46 @@ export function TodoListDisplay({ todos }: { todos: any[] }) { {/* Inline expanded list */} {!isCollapsed && (
    - {todos.map((todo: any, idx: number) => { - const icon = getTodoIcon(todo.status as TodoStatus) + {todos.map((todo, idx: number) => { + const todoStatus = (todo.status as TodoStatus) ?? "pending" + const icon = getTodoIcon(todoStatus) + const isClickable = Boolean(todo.subtaskId && onSubtaskClick) + const subtaskById = + subtaskDetails && todo.subtaskId + ? subtaskDetails.find((s) => s.id === todo.subtaskId) + : undefined + const displayTokens = todo.tokens ?? subtaskById?.tokens + const displayCost = todo.cost ?? subtaskById?.cost + const shouldShowCost = typeof displayTokens === "number" && typeof displayCost === "number" + return (
  • (itemRefs.current[idx] = el)} className={cn( "font-light flex flex-row gap-2 items-start min-h-[20px] leading-normal mb-2", - todo.status === "in_progress" && "text-vscode-charts-yellow", - todo.status !== "in_progress" && todo.status !== "completed" && "opacity-60", + todoStatus === "in_progress" && "text-vscode-charts-yellow", + todoStatus !== "in_progress" && todoStatus !== "completed" && "opacity-60", )}> {icon} - {todo.content} + onSubtaskClick?.(todo.subtaskId as string) : undefined + }> + {todo.content} + + {/* Token count and cost display */} + {shouldShowCost && ( + + + {formatLargeNumber(displayTokens)} + + + ${displayCost.toFixed(2)} + + + )}
    • ) })} diff --git a/webview-ui/src/components/chat/__tests__/TodoListDisplay.spec.tsx b/webview-ui/src/components/chat/__tests__/TodoListDisplay.spec.tsx new file mode 100644 index 00000000000..8d0e16494b7 --- /dev/null +++ b/webview-ui/src/components/chat/__tests__/TodoListDisplay.spec.tsx @@ -0,0 +1,229 @@ +import { describe, it, expect, vi } from "vitest" +import { render, screen, fireEvent } from "@testing-library/react" + +import { TodoListDisplay } from "../TodoListDisplay" +import type { SubtaskDetail } from "@src/types/subtasks" + +// Mock i18next +vi.mock("i18next", () => ({ + t: (key: string, options?: Record) => { + if (key === "chat:todo.complete") return `${options?.total} to-dos done` + if (key === "chat:todo.partial") return `${options?.completed} of ${options?.total} to-dos done` + return key + }, +})) + +// Mock format utility +vi.mock("@src/utils/format", () => ({ + formatLargeNumber: (num: number) => { + if (num >= 1e3) return `${(num / 1e3).toFixed(1)}k` + return num.toString() + }, +})) + +describe("TodoListDisplay", () => { + const baseTodos = [ + { id: "1", content: "Task 1: Change background colour", status: "completed", subtaskId: "subtask-1" }, + { id: "2", content: "Task 2: Add timestamp to bottom", status: "completed", subtaskId: "subtask-2" }, + { id: "3", content: "Task 3: Pending task", status: "pending" }, + ] + + const subtaskDetails: SubtaskDetail[] = [ + { + id: "subtask-1", + name: "Task 1: Change background colour", + tokens: 95400, + cost: 0.22, + status: "completed", + hasNestedChildren: false, + }, + { + id: "subtask-2", + name: "Task 2: Add timestamp to bottom", + tokens: 95000, + cost: 0.24, + status: "completed", + hasNestedChildren: false, + }, + ] + + describe("basic rendering", () => { + it("should render nothing when todos is empty", () => { + const { container } = render() + expect(container.firstChild).toBeNull() + }) + + it("should render collapsed view by default", () => { + render() + // Should show the first incomplete task in collapsed view + expect(screen.getByText("Task 3: Pending task")).toBeInTheDocument() + }) + + it("should expand when header is clicked", () => { + render() + const header = screen.getByText("Task 3: Pending task") + fireEvent.click(header) + + // After expanding, should show all tasks + expect(screen.getByText("Task 1: Change background colour")).toBeInTheDocument() + expect(screen.getByText("Task 2: Add timestamp to bottom")).toBeInTheDocument() + expect(screen.getByText("Task 3: Pending task")).toBeInTheDocument() + }) + + it("should show completion count when all tasks are complete", () => { + const completedTodos = [ + { id: "1", content: "Task 1", status: "completed" }, + { id: "2", content: "Task 2", status: "completed" }, + ] + render() + expect(screen.getByText("2 to-dos done")).toBeInTheDocument() + }) + }) + + describe("subtask cost display", () => { + it("should display tokens and cost when subtaskDetails are provided and todo.subtaskId matches", () => { + render() + + // Expand to see the items + const header = screen.getByText("Task 3: Pending task") + fireEvent.click(header) + + // Check for formatted token counts + expect(screen.getByText("95.4k")).toBeInTheDocument() + expect(screen.getByText("95.0k")).toBeInTheDocument() + + // Check for costs + expect(screen.getByText("$0.22")).toBeInTheDocument() + expect(screen.getByText("$0.24")).toBeInTheDocument() + }) + + it("should not display tokens/cost for todos without subtaskId", () => { + render() + + // Expand to see the items + const header = screen.getByText("Task 3: Pending task") + fireEvent.click(header) + + // The pending task has no subtaskId, should not show cost + const listItems = screen.getAllByRole("listitem") + const pendingItem = listItems.find((item: HTMLElement) => + item.textContent?.includes("Task 3: Pending task"), + ) + expect(pendingItem).toBeDefined() + expect(pendingItem?.textContent).not.toContain("$") + }) + + it("should not display tokens/cost when subtaskDetails is undefined", () => { + render() + + // Expand to see the items + const header = screen.getByText("Task 3: Pending task") + fireEvent.click(header) + + // No cost should be displayed + expect(screen.queryByText("$0.22")).not.toBeInTheDocument() + expect(screen.queryByText("$0.24")).not.toBeInTheDocument() + }) + + it("should not display tokens/cost when subtaskDetails is empty array", () => { + render() + + // Expand to see the items + const header = screen.getByText("Task 3: Pending task") + fireEvent.click(header) + + // No cost should be displayed + expect(screen.queryByText("$0.22")).not.toBeInTheDocument() + }) + }) + + describe("direct subtask linking", () => { + it("should use todo.tokens and todo.cost when provided (no subtaskDetails required)", () => { + const todosWithDirectCost = [ + { + id: "1", + content: "Task 1: Change background colour", + status: "completed", + subtaskId: "subtask-1", + tokens: 95400, + cost: 0.22, + }, + ] + render() + + // Expand + const header = screen.getByText("1 to-dos done") + fireEvent.click(header) + + expect(screen.getByText("95.4k")).toBeInTheDocument() + expect(screen.getByText("$0.22")).toBeInTheDocument() + }) + + it("should fall back to subtaskDetails by ID when todo.tokens/cost are missing", () => { + const todosMissingCostFields = [ + { + id: "1", + content: "Task 1: Change background colour", + status: "completed", + subtaskId: "subtask-1", + }, + ] + render() + + // Expand + const header = screen.getByText("1 to-dos done") + fireEvent.click(header) + + expect(screen.getByText("95.4k")).toBeInTheDocument() + expect(screen.getByText("$0.22")).toBeInTheDocument() + }) + }) + + describe("click handler", () => { + it("should call onSubtaskClick when a todo with subtaskId is clicked", () => { + const onSubtaskClick = vi.fn() + render( + , + ) + + // Expand + const header = screen.getByText("Task 3: Pending task") + fireEvent.click(header) + + // Click on first matched todo + const task1 = screen.getByText("Task 1: Change background colour") + fireEvent.click(task1) + + expect(onSubtaskClick).toHaveBeenCalledWith("subtask-1") + }) + + it("should not call onSubtaskClick when a todo does not have subtaskId", () => { + const onSubtaskClick = vi.fn() + render( + , + ) + + // Expand + const header = screen.getByText("Task 3: Pending task") + fireEvent.click(header) + + // Click on unmatched todo + const task3 = screen.getByText("Task 3: Pending task") + fireEvent.click(task3) + + expect(onSubtaskClick).not.toHaveBeenCalled() + }) + + it("should not be clickable when onSubtaskClick is not provided", () => { + render() + + // Expand + const header = screen.getByText("Task 3: Pending task") + fireEvent.click(header) + + // Task should be present but not have hover:underline class behavior + const task1 = screen.getByText("Task 1: Change background colour") + expect(task1.className).not.toContain("cursor-pointer") + }) + }) +}) diff --git a/webview-ui/src/i18n/locales/ca/common.json b/webview-ui/src/i18n/locales/ca/common.json index 56e9a3745ef..c51cef26072 100644 --- a/webview-ui/src/i18n/locales/ca/common.json +++ b/webview-ui/src/i18n/locales/ca/common.json @@ -108,7 +108,10 @@ "awaiting_child": "Esperant la tasca filla {{childId}}" }, "costs": { - "own": "Propi", + "own": "Task", "subtasks": "Subtasques" + }, + "stats": { + "lines": "Línies" } } diff --git a/webview-ui/src/i18n/locales/de/common.json b/webview-ui/src/i18n/locales/de/common.json index ab8bd6d2401..83c2e746088 100644 --- a/webview-ui/src/i18n/locales/de/common.json +++ b/webview-ui/src/i18n/locales/de/common.json @@ -108,7 +108,10 @@ "awaiting_child": "Warte auf Unteraufgabe {{childId}}" }, "costs": { - "own": "Eigen", + "own": "Task", "subtasks": "Unteraufgaben" + }, + "stats": { + "lines": "Zeilen" } } diff --git a/webview-ui/src/i18n/locales/en/chat.json b/webview-ui/src/i18n/locales/en/chat.json index 3ab2c037af2..36c19e54efd 100644 --- a/webview-ui/src/i18n/locales/en/chat.json +++ b/webview-ui/src/i18n/locales/en/chat.json @@ -6,6 +6,7 @@ "collapse": "Collapse task", "seeMore": "See more", "seeLess": "See less", + "lineChanges": "Lines:", "tokens": "Tokens", "cache": "Cache", "apiCost": "API Cost", diff --git a/webview-ui/src/i18n/locales/en/common.json b/webview-ui/src/i18n/locales/en/common.json index 981eaeec755..f830b38c412 100644 --- a/webview-ui/src/i18n/locales/en/common.json +++ b/webview-ui/src/i18n/locales/en/common.json @@ -108,7 +108,10 @@ "awaiting_child": "Awaiting child task {{childId}}" }, "costs": { - "own": "Own", + "own": "Task", "subtasks": "Subtasks" + }, + "stats": { + "lines": "Lines" } } diff --git a/webview-ui/src/i18n/locales/es/common.json b/webview-ui/src/i18n/locales/es/common.json index 03455b7cadc..e64507350e6 100644 --- a/webview-ui/src/i18n/locales/es/common.json +++ b/webview-ui/src/i18n/locales/es/common.json @@ -108,7 +108,10 @@ "awaiting_child": "Esperando tarea secundaria {{childId}}" }, "costs": { - "own": "Propio", + "own": "Task", "subtasks": "Subtareas" + }, + "stats": { + "lines": "Líneas" } } diff --git a/webview-ui/src/i18n/locales/fr/common.json b/webview-ui/src/i18n/locales/fr/common.json index def93ad6c5e..bb6e658bd74 100644 --- a/webview-ui/src/i18n/locales/fr/common.json +++ b/webview-ui/src/i18n/locales/fr/common.json @@ -108,7 +108,10 @@ "awaiting_child": "En attente de la tâche enfant {{childId}}" }, "costs": { - "own": "Propre", + "own": "Task", "subtasks": "Sous-tâches" + }, + "stats": { + "lines": "Lignes" } } diff --git a/webview-ui/src/i18n/locales/hi/common.json b/webview-ui/src/i18n/locales/hi/common.json index 076530e6b02..e3104e9d59c 100644 --- a/webview-ui/src/i18n/locales/hi/common.json +++ b/webview-ui/src/i18n/locales/hi/common.json @@ -108,7 +108,10 @@ "awaiting_child": "चाइल्ड कार्य {{childId}} की प्रतीक्षा में" }, "costs": { - "own": "स्वयं", + "own": "Task", "subtasks": "उपकार्य" + }, + "stats": { + "lines": "पंक्तियाँ" } } diff --git a/webview-ui/src/i18n/locales/id/common.json b/webview-ui/src/i18n/locales/id/common.json index a65295f28d4..6557610847d 100644 --- a/webview-ui/src/i18n/locales/id/common.json +++ b/webview-ui/src/i18n/locales/id/common.json @@ -108,7 +108,10 @@ "awaiting_child": "Menunggu tugas anak {{childId}}" }, "costs": { - "own": "Sendiri", + "own": "Task", "subtasks": "Subtugas" + }, + "stats": { + "lines": "Baris" } } diff --git a/webview-ui/src/i18n/locales/it/common.json b/webview-ui/src/i18n/locales/it/common.json index 9b801628f49..efb134f75eb 100644 --- a/webview-ui/src/i18n/locales/it/common.json +++ b/webview-ui/src/i18n/locales/it/common.json @@ -108,7 +108,10 @@ "awaiting_child": "In attesa dell'attività figlia {{childId}}" }, "costs": { - "own": "Proprio", + "own": "Task", "subtasks": "Sottoattività" + }, + "stats": { + "lines": "Linee" } } diff --git a/webview-ui/src/i18n/locales/ja/common.json b/webview-ui/src/i18n/locales/ja/common.json index b3b9d462e07..d7502d3ddc7 100644 --- a/webview-ui/src/i18n/locales/ja/common.json +++ b/webview-ui/src/i18n/locales/ja/common.json @@ -108,7 +108,10 @@ "awaiting_child": "子タスク{{childId}}を待機中" }, "costs": { - "own": "自身", + "own": "Task", "subtasks": "サブタスク" + }, + "stats": { + "lines": "行" } } diff --git a/webview-ui/src/i18n/locales/ko/common.json b/webview-ui/src/i18n/locales/ko/common.json index d7120e2520d..c0be2ff9159 100644 --- a/webview-ui/src/i18n/locales/ko/common.json +++ b/webview-ui/src/i18n/locales/ko/common.json @@ -108,7 +108,10 @@ "awaiting_child": "하위 작업 {{childId}} 대기 중" }, "costs": { - "own": "자체", + "own": "Task", "subtasks": "하위작업" + }, + "stats": { + "lines": "라인" } } diff --git a/webview-ui/src/i18n/locales/nl/common.json b/webview-ui/src/i18n/locales/nl/common.json index ec6cf89ccb5..a0c74f6da59 100644 --- a/webview-ui/src/i18n/locales/nl/common.json +++ b/webview-ui/src/i18n/locales/nl/common.json @@ -108,7 +108,10 @@ "awaiting_child": "Wachten op kindtaak {{childId}}" }, "costs": { - "own": "Eigen", + "own": "Task", "subtasks": "Subtaken" + }, + "stats": { + "lines": "Regels" } } diff --git a/webview-ui/src/i18n/locales/pl/common.json b/webview-ui/src/i18n/locales/pl/common.json index 419aa83af1e..a7410b91cbf 100644 --- a/webview-ui/src/i18n/locales/pl/common.json +++ b/webview-ui/src/i18n/locales/pl/common.json @@ -108,7 +108,10 @@ "awaiting_child": "Oczekiwanie na zadanie podrzędne {{childId}}" }, "costs": { - "own": "Własne", + "own": "Task", "subtasks": "Podzadania" + }, + "stats": { + "lines": "Linie" } } diff --git a/webview-ui/src/i18n/locales/pt-BR/common.json b/webview-ui/src/i18n/locales/pt-BR/common.json index 4990796976f..d186e331227 100644 --- a/webview-ui/src/i18n/locales/pt-BR/common.json +++ b/webview-ui/src/i18n/locales/pt-BR/common.json @@ -108,7 +108,10 @@ "awaiting_child": "Aguardando tarefa filha {{childId}}" }, "costs": { - "own": "Próprio", + "own": "Task", "subtasks": "Subtarefas" + }, + "stats": { + "lines": "Linhas" } } diff --git a/webview-ui/src/i18n/locales/ru/common.json b/webview-ui/src/i18n/locales/ru/common.json index f66384a6937..3d2306c589c 100644 --- a/webview-ui/src/i18n/locales/ru/common.json +++ b/webview-ui/src/i18n/locales/ru/common.json @@ -108,7 +108,10 @@ "awaiting_child": "Ожидание дочерней задачи {{childId}}" }, "costs": { - "own": "Собственные", + "own": "Task", "subtasks": "Подзадачи" + }, + "stats": { + "lines": "Строки" } } diff --git a/webview-ui/src/i18n/locales/tr/common.json b/webview-ui/src/i18n/locales/tr/common.json index db9e991cd58..deddd91f14f 100644 --- a/webview-ui/src/i18n/locales/tr/common.json +++ b/webview-ui/src/i18n/locales/tr/common.json @@ -108,7 +108,10 @@ "awaiting_child": "{{childId}} alt görevi bekleniyor" }, "costs": { - "own": "Kendi", + "own": "Task", "subtasks": "Alt görevler" + }, + "stats": { + "lines": "Satırlar" } } diff --git a/webview-ui/src/i18n/locales/vi/common.json b/webview-ui/src/i18n/locales/vi/common.json index 57eb31fafa4..92a8f2facf2 100644 --- a/webview-ui/src/i18n/locales/vi/common.json +++ b/webview-ui/src/i18n/locales/vi/common.json @@ -108,7 +108,10 @@ "awaiting_child": "Đang chờ nhiệm vụ con {{childId}}" }, "costs": { - "own": "Riêng", + "own": "Task", "subtasks": "Nhiệm vụ con" + }, + "stats": { + "lines": "Dòng" } } diff --git a/webview-ui/src/i18n/locales/zh-CN/common.json b/webview-ui/src/i18n/locales/zh-CN/common.json index 10df0893334..653df60972e 100644 --- a/webview-ui/src/i18n/locales/zh-CN/common.json +++ b/webview-ui/src/i18n/locales/zh-CN/common.json @@ -108,7 +108,10 @@ "awaiting_child": "等待子任务 {{childId}}" }, "costs": { - "own": "自身", + "own": "Task", "subtasks": "子任务" + }, + "stats": { + "lines": "行" } } diff --git a/webview-ui/src/i18n/locales/zh-TW/common.json b/webview-ui/src/i18n/locales/zh-TW/common.json index da47dec72bd..2b20cb4eed2 100644 --- a/webview-ui/src/i18n/locales/zh-TW/common.json +++ b/webview-ui/src/i18n/locales/zh-TW/common.json @@ -108,7 +108,10 @@ "awaiting_child": "等待子工作 {{childId}}" }, "costs": { - "own": "自身", + "own": "Task", "subtasks": "子工作" + }, + "stats": { + "lines": "行" } } diff --git a/webview-ui/src/types/subtasks.ts b/webview-ui/src/types/subtasks.ts new file mode 100644 index 00000000000..a4cd97ed4f9 --- /dev/null +++ b/webview-ui/src/types/subtasks.ts @@ -0,0 +1,13 @@ +export type SubtaskDetail = { + /** Task ID */ + id: string + /** First 50 chars of task description */ + name: string + /** tokensIn + tokensOut */ + tokens: number + /** Aggregated total cost */ + cost: number + status: "active" | "completed" | "delegated" + /** Has its own subtasks */ + hasNestedChildren: boolean +} diff --git a/webview-ui/src/utils/__tests__/taskCostBreakdown.spec.ts b/webview-ui/src/utils/__tests__/taskCostBreakdown.spec.ts new file mode 100644 index 00000000000..fe7f00771fb --- /dev/null +++ b/webview-ui/src/utils/__tests__/taskCostBreakdown.spec.ts @@ -0,0 +1,30 @@ +import { describe, it, expect } from "vitest" + +import { computeTaskCostsIncludingSubtasks, getTaskHeaderCostTooltipData } from "@src/utils/taskCostBreakdown" + +describe("taskCostBreakdown", () => { + it("sums subtask line-item costs (micros) and produces a stable total", () => { + const result = computeTaskCostsIncludingSubtasks(0.05, [0.16, 0.13]) + + expect(result.ownCostCents).toBe(5) + expect(result.subtasksCostCents).toBe(29) + expect(result.totalCostIncludingSubtasksCents).toBe(34) + expect(result.totalCostIncludingSubtasks).toBeCloseTo(0.34, 10) + }) + + it("prefers derived subtask sum over provided breakdown/aggregatedCost when details are available", () => { + const data = getTaskHeaderCostTooltipData({ + ownCost: 0.05, + aggregatedCost: 0.09, + hasSubtasksProp: true, + costBreakdownProp: "Own: $0.05 + Subtasks: $0.09", + subtaskCosts: [0.16, 0.13], + labels: { own: "Own", subtasks: "Subtasks" }, + }) + + expect(data.displayTotalCost).toBeCloseTo(0.34, 10) + expect(data.displayCostBreakdown).toBe("Own: $0.05 + Subtasks: $0.29") + expect(data.hasSubtasks).toBe(true) + expect(data.hasAnyCost).toBe(true) + }) +}) diff --git a/webview-ui/src/utils/taskCostBreakdown.ts b/webview-ui/src/utils/taskCostBreakdown.ts new file mode 100644 index 00000000000..514246e50eb --- /dev/null +++ b/webview-ui/src/utils/taskCostBreakdown.ts @@ -0,0 +1,104 @@ +import { formatCostBreakdown } from "@src/utils/costFormatting" + +const MICROS_PER_DOLLAR = 1_000_000 +const MICROS_PER_CENT = 10_000 + +function dollarsToMicros(amount: number): number { + if (typeof amount !== "number" || !Number.isFinite(amount) || amount <= 0) { + return 0 + } + return Math.round(amount * MICROS_PER_DOLLAR) +} + +function microsToDollars(micros: number): number { + return micros / MICROS_PER_DOLLAR +} + +export interface TaskCostsIncludingSubtasks { + ownCost: number + ownCostMicros: number + ownCostCents: number + + subtasksCost: number + subtasksCostMicros: number + subtasksCostCents: number + + totalCostIncludingSubtasks: number + totalCostIncludingSubtasksMicros: number + totalCostIncludingSubtasksCents: number +} + +/** + * Computes task costs using integer micros for stable aggregation. + * + * Note: we only have access to floating-dollar amounts in the webview. + * Converting to micros and summing avoids most floating point drift. + */ +export function computeTaskCostsIncludingSubtasks(ownCost: number, subtaskCosts: number[]): TaskCostsIncludingSubtasks { + const ownCostMicros = dollarsToMicros(ownCost) + const subtasksCostMicros = (subtaskCosts ?? []).reduce((sum, cost) => sum + dollarsToMicros(cost), 0) + const totalCostIncludingSubtasksMicros = ownCostMicros + subtasksCostMicros + + const ownCostCents = Math.round(ownCostMicros / MICROS_PER_CENT) + const subtasksCostCents = Math.round(subtasksCostMicros / MICROS_PER_CENT) + const totalCostIncludingSubtasksCents = Math.round(totalCostIncludingSubtasksMicros / MICROS_PER_CENT) + + return { + ownCost: microsToDollars(ownCostMicros), + ownCostMicros, + ownCostCents, + subtasksCost: microsToDollars(subtasksCostMicros), + subtasksCostMicros, + subtasksCostCents, + totalCostIncludingSubtasks: microsToDollars(totalCostIncludingSubtasksMicros), + totalCostIncludingSubtasksMicros, + totalCostIncludingSubtasksCents, + } +} + +export interface TaskHeaderCostTooltipData { + /** Total cost to display (includes subtasks when details provided). */ + displayTotalCost: number + /** Breakdown string to show in tooltip, if subtasks exist. */ + displayCostBreakdown?: string + /** Whether the UI should show the "includes subtasks" marker. */ + hasSubtasks: boolean + /** Whether there is any cost to render. */ + hasAnyCost: boolean +} + +/** + * Cost display logic for TaskHeader tooltip. + * + * When subtask details are available, this derives subtasks cost as the sum of + * subtask line-item totals (same source as the accordion list), rather than + * trusting any derived deltas. + */ +export function getTaskHeaderCostTooltipData(params: { + ownCost: number + aggregatedCost?: number + hasSubtasksProp?: boolean + costBreakdownProp?: string + subtaskCosts?: number[] + labels: { own: string; subtasks: string } +}): TaskHeaderCostTooltipData { + const { ownCost, aggregatedCost, hasSubtasksProp, costBreakdownProp, subtaskCosts, labels } = params + + const computed = computeTaskCostsIncludingSubtasks(ownCost, subtaskCosts ?? []) + const hasComputedSubtasks = computed.subtasksCostCents > 0 + const hasSubtasks = !!hasSubtasksProp || hasComputedSubtasks + + const displayTotalCost = hasComputedSubtasks ? computed.totalCostIncludingSubtasks : (aggregatedCost ?? ownCost) + const displayCostBreakdown = hasComputedSubtasks + ? formatCostBreakdown(computed.ownCost, computed.subtasksCost, labels) + : costBreakdownProp + + const hasAnyCost = typeof displayTotalCost === "number" && Number.isFinite(displayTotalCost) && displayTotalCost > 0 + + return { + displayTotalCost, + displayCostBreakdown, + hasSubtasks, + hasAnyCost, + } +}