From 3900b3c2783800c7bb10548d91913a55c568f795 Mon Sep 17 00:00:00 2001 From: Toray Altas Date: Tue, 20 Jan 2026 16:46:40 -0500 Subject: [PATCH] fix: preserve todo metadata across subtask updates Carry forward subtaskId/cost/tokens/line changes when the LLM rewrites todo text, including sequential updates.\n\nIncludes [TODO-DEBUG] instrumentation across extension/webview and regression tests. --- .../presentAssistantMessage.ts | 5 + src/core/tools/UpdateTodoListTool.ts | 636 +++++++++++++++++- .../__tests__/updateTodoListTool.spec.ts | 483 ++++++++++++- src/core/webview/ClineProvider.ts | 99 ++- ...openParentFromDelegation.writeback.spec.ts | 237 +++++++ .../__tests__/aggregateTaskCosts.spec.ts | 68 ++ src/shared/todo.ts | 69 +- webview-ui/src/components/chat/ChatRow.tsx | 29 +- webview-ui/src/components/chat/ChatView.tsx | 10 + .../src/components/chat/TodoChangeDisplay.tsx | 41 +- .../src/components/chat/TodoListDisplay.tsx | 52 ++ 11 files changed, 1662 insertions(+), 67 deletions(-) create mode 100644 src/core/webview/__tests__/ClineProvider.reopenParentFromDelegation.writeback.spec.ts diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index 693327a022..7bc29ac95a 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -881,6 +881,11 @@ export async function presentAssistantMessage(cline: Task) { }) break case "update_todo_list": + console.log("[TODO-DEBUG]", "presentAssistantMessage dispatching update_todo_list", { + toolUseId: (block as any).id, + partial: block.partial, + params: (block as any).params, + }) await updateTodoListTool.handle(cline, block as ToolUse<"update_todo_list">, { askApproval, handleError, diff --git a/src/core/tools/UpdateTodoListTool.ts b/src/core/tools/UpdateTodoListTool.ts index 795903bb38..1f5467034a 100644 --- a/src/core/tools/UpdateTodoListTool.ts +++ b/src/core/tools/UpdateTodoListTool.ts @@ -2,8 +2,10 @@ 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 fs from "fs" +import os from "os" +import path from "path" import { TodoItem, TodoStatus, todoStatusSchema } from "@roo-code/types" import { getLatestTodo } from "../../shared/todo" @@ -23,11 +25,54 @@ export class UpdateTodoListTool extends BaseTool<"update_todo_list"> { } async execute(params: UpdateTodoListParams, task: Task, callbacks: ToolCallbacks): Promise { + console.log("[TODO-DEBUG] execute() STEP 0: ENTERED", { + tool: "update_todo_list", + paramsTodosType: typeof params?.todos, + paramsTodosLength: typeof params?.todos === "string" ? params.todos.length : undefined, + }) const { pushToolResult, handleError, askApproval, toolProtocol } = callbacks try { + const summarizeTodoForDebug = (t: TodoItem | undefined) => { + if (!t) return undefined + return { + id: typeof t.id === "string" ? t.id : undefined, + status: typeof t.status === "string" ? t.status : undefined, + content: typeof t.content === "string" ? t.content.slice(0, 120) : undefined, + subtaskId: typeof t.subtaskId === "string" ? t.subtaskId : undefined, + tokens: typeof t.tokens === "number" ? t.tokens : undefined, + cost: typeof t.cost === "number" ? t.cost : undefined, + added: typeof t.added === "number" ? t.added : undefined, + removed: typeof t.removed === "number" ? t.removed : undefined, + } + } + + const shouldTodoDebugLog = + process.env.ROO_DEBUG_TODO_METADATA === "1" || + process.env.ROO_DEBUG_TODO_METADATA === "true" || + process.env.ROO_CLI_DEBUG_LOG === "1" + console.log("[TODO-DEBUG] execute() STEP 1: computed debug flags", { + shouldTodoDebugLog, + ROO_DEBUG_TODO_METADATA: process.env.ROO_DEBUG_TODO_METADATA, + ROO_CLI_DEBUG_LOG: process.env.ROO_CLI_DEBUG_LOG, + toolProtocol, + }) + const previousFromMemory = getTodoListForTask(task) + console.log("[TODO-DEBUG] execute() STEP 2: previous todos from memory", { + previousFromMemoryCount: Array.isArray(previousFromMemory) ? previousFromMemory.length : 0, + previousFromMemoryPreview: Array.isArray(previousFromMemory) + ? previousFromMemory.slice(0, 10).map((t) => summarizeTodoForDebug(t)) + : undefined, + }) + const previousFromHistory = getLatestTodo(task.clineMessages) as unknown as TodoItem[] | undefined + console.log("[TODO-DEBUG] execute() STEP 3: previous todos from history", { + previousFromHistoryCount: Array.isArray(previousFromHistory) ? previousFromHistory.length : 0, + previousFromHistoryPreview: Array.isArray(previousFromHistory) + ? previousFromHistory.slice(0, 10).map((t) => summarizeTodoForDebug(t)) + : undefined, + }) const historyHasMetadata = Array.isArray(previousFromHistory) && @@ -39,6 +84,21 @@ export class UpdateTodoListTool extends BaseTool<"update_todo_list"> { t?.added !== undefined || t?.removed !== undefined, ) + console.log("[TODO-DEBUG] execute() STEP 4: analyzed history metadata", { + historyHasMetadata, + historyHasSubtaskId: Array.isArray(previousFromHistory) + ? previousFromHistory.some((t) => typeof t?.subtaskId === "string") + : false, + historyHasTokens: Array.isArray(previousFromHistory) + ? previousFromHistory.some((t) => typeof t?.tokens === "number") + : false, + historyHasCost: Array.isArray(previousFromHistory) + ? previousFromHistory.some((t) => typeof t?.cost === "number") + : false, + historyHasLineChanges: Array.isArray(previousFromHistory) + ? previousFromHistory.some((t) => typeof t?.added === "number" || typeof t?.removed === "number") + : false, + }) const previousTodos: TodoItem[] = (previousFromMemory?.length ?? 0) === 0 @@ -48,25 +108,109 @@ export class UpdateTodoListTool extends BaseTool<"update_todo_list"> { : historyHasMetadata ? (previousFromHistory ?? []) : (previousFromMemory ?? []) + console.log("[TODO-DEBUG] execute() STEP 5: selected previousTodos", { + selectedPreviousTodosCount: Array.isArray(previousTodos) ? previousTodos.length : 0, + selectedPreviousTodosWithSubtaskIdCount: Array.isArray(previousTodos) + ? previousTodos.filter((t) => typeof t?.subtaskId === "string").length + : 0, + selectedPreviousTodosPreview: Array.isArray(previousTodos) + ? previousTodos.slice(0, 10).map((t) => summarizeTodoForDebug(t)) + : undefined, + }) const todosRaw = params.todos + if (shouldTodoDebugLog) { + console.log("[TODO-DEBUG]", "UpdateTodoListTool.execute() received params.todos", { + tool: "update_todo_list", + todosRawType: typeof todosRaw, + todosRawLength: typeof todosRaw === "string" ? todosRaw.length : undefined, + todosRawPreview: typeof todosRaw === "string" ? todosRaw.slice(0, 500) : undefined, + }) + console.log("[TODO-DEBUG]", "UpdateTodoListTool.execute() previousTodos summary", { + previousFromMemoryCount: Array.isArray(previousFromMemory) ? previousFromMemory.length : 0, + previousFromHistoryCount: Array.isArray(previousFromHistory) ? previousFromHistory.length : 0, + historyHasMetadata, + selectedPreviousTodosCount: Array.isArray(previousTodos) ? previousTodos.length : 0, + previousTodosWithSubtaskIdCount: Array.isArray(previousTodos) + ? previousTodos.filter((t) => typeof t?.subtaskId === "string").length + : 0, + }) + } let todos: TodoItem[] - try { - todos = parseMarkdownChecklist(todosRaw || "") - } catch { + const jsonParseResult = tryParseTodoItemsJson(todosRaw) + if (jsonParseResult.parsed) { + todos = jsonParseResult.parsed + console.log("[TODO-DEBUG] execute() STEP 6: parsed todos via JSON", { + parsedCount: todos.length, + parsedPreview: todos.slice(0, 10).map((t) => summarizeTodoForDebug(t)), + }) + if (shouldTodoDebugLog) { + console.log("[TODO-DEBUG]", "UpdateTodoListTool.execute() parsed todos from JSON", { + parsedCount: todos.length, + hasAnySubtaskId: todos.some((t) => typeof t?.subtaskId === "string"), + subtaskIds: todos.map((t) => t?.subtaskId).filter(Boolean), + }) + } + } else if (jsonParseResult.error) { + console.log("[TODO-DEBUG] execute() STEP 6: JSON parse/validate error", { + error: jsonParseResult.error, + todosRawPreview: typeof todosRaw === "string" ? todosRaw.slice(0, 500) : undefined, + }) 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 || "") + console.log("[TODO-DEBUG] execute() STEP 6: parsed todos via markdown checklist", { + parsedCount: todos.length, + parsedPreview: todos.slice(0, 10).map((t) => summarizeTodoForDebug(t)), + }) + if (shouldTodoDebugLog) { + console.log("[TODO-DEBUG]", "UpdateTodoListTool.execute() parsed todos from markdown checklist", { + parsedCount: todos.length, + hasAnySubtaskId: todos.some((t) => typeof t?.subtaskId === "string"), + }) + } } // 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) + // NOTE: Instrumentation is enabled here (once per tool execute) to detect metadata-preservation failures. + console.log("[TODO-DEBUG] execute() STEP 7: about to call preserveTodoMetadata", { + nextTodosCount: Array.isArray(todos) ? todos.length : 0, + previousTodosCount: Array.isArray(previousTodos) ? previousTodos.length : 0, + enableInstrumentation: true, + }) + const todosWithPreservedMetadata = preserveTodoMetadata(todos, previousTodos, { + enableInstrumentation: true, + }) + console.log("[TODO-DEBUG] execute() STEP 8: returned from preserveTodoMetadata", { + resultCount: todosWithPreservedMetadata.length, + resultPreview: todosWithPreservedMetadata.slice(0, 10).map((t) => summarizeTodoForDebug(t)), + }) + if (shouldTodoDebugLog) { + console.log("[TODO-DEBUG]", "UpdateTodoListTool.execute() after preserveTodoMetadata()", { + nextTodosCount: todosWithPreservedMetadata.length, + todosWithSubtaskIdCount: todosWithPreservedMetadata.filter((t) => typeof t?.subtaskId === "string") + .length, + subtaskIds: todosWithPreservedMetadata.map((t) => t?.subtaskId).filter(Boolean), + hasAnyTokens: todosWithPreservedMetadata.some((t) => typeof t?.tokens === "number"), + hasAnyCost: todosWithPreservedMetadata.some((t) => typeof t?.cost === "number"), + hasAnyLineChanges: todosWithPreservedMetadata.some( + (t) => typeof t?.added === "number" || typeof t?.removed === "number", + ), + }) + } - const { valid, error } = validateTodos(todos) + const { valid, error } = validateTodos(todosWithPreservedMetadata) + console.log("[TODO-DEBUG] execute() STEP 9: validateTodos", { + valid, + error, + }) if (!valid) { task.consecutiveMistakeCount++ task.recordToolError("update_todo_list") @@ -85,27 +229,76 @@ export class UpdateTodoListTool extends BaseTool<"update_todo_list"> { added: t.added, removed: t.removed, })) + console.log("[TODO-DEBUG] execute() STEP 10: normalizedTodos (pre-approval)", { + normalizedCount: normalizedTodos.length, + normalizedPreview: normalizedTodos.slice(0, 10).map((t) => summarizeTodoForDebug(t)), + }) + if (shouldTodoDebugLog) { + console.log("[TODO-DEBUG]", "UpdateTodoListTool.execute() normalizedTodos (pre-approval)", { + normalizedCount: normalizedTodos.length, + todosWithSubtaskIdCount: normalizedTodos.filter((t) => typeof t?.subtaskId === "string").length, + subtaskIds: normalizedTodos.map((t) => t?.subtaskId).filter(Boolean), + }) + } const approvalMsg = JSON.stringify({ tool: "updateTodoList", todos: normalizedTodos, }) - approvedTodoList = cloneDeep(normalizedTodos) + // TodoItem is a flat object shape; a shallow copy is sufficient here. + approvedTodoList = normalizedTodos.map((t) => ({ ...t })) + console.log("[TODO-DEBUG] execute() STEP 11: asking approval", { + approvalPayloadLength: approvalMsg.length, + normalizedCount: normalizedTodos.length, + }) const didApprove = await askApproval("tool", approvalMsg) + console.log("[TODO-DEBUG] execute() STEP 12: approval result", { + didApprove, + }) if (!didApprove) { + console.log("[TODO-DEBUG] execute() STEP 13: user declined; returning", {}) pushToolResult("User declined to update the todoList.") return } const isTodoListChanged = approvedTodoList !== undefined && JSON.stringify(normalizedTodos) !== JSON.stringify(approvedTodoList) + console.log("[TODO-DEBUG] execute() STEP 14: checked approval UI edits", { + isTodoListChanged, + }) if (isTodoListChanged) { normalizedTodos = approvedTodoList ?? [] + console.log("[TODO-DEBUG] execute() STEP 15: using user-edited todos", { + editedCount: normalizedTodos.length, + editedPreview: normalizedTodos.slice(0, 10).map((t) => summarizeTodoForDebug(t)), + }) + if (shouldTodoDebugLog) { + console.log("[TODO-DEBUG]", "UpdateTodoListTool.execute() user edited todos in approval UI", { + editedCount: normalizedTodos.length, + todosWithSubtaskIdCount: normalizedTodos.filter((t) => typeof t?.subtaskId === "string").length, + subtaskIds: normalizedTodos.map((t) => t?.subtaskId).filter(Boolean), + }) + } // 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) + // NOTE: Do not instrument here to avoid double-logging within the same update. + console.log("[TODO-DEBUG] execute() STEP 16: about to re-call preserveTodoMetadata (user-edited)", { + enableInstrumentation: false, + }) + normalizedTodos = preserveTodoMetadata(normalizedTodos, previousTodos, { enableInstrumentation: false }) + console.log("[TODO-DEBUG] execute() STEP 17: returned from re-preserve (user-edited)", { + normalizedCount: normalizedTodos.length, + normalizedPreview: normalizedTodos.slice(0, 10).map((t) => summarizeTodoForDebug(t)), + }) + if (shouldTodoDebugLog) { + console.log("[TODO-DEBUG]", "UpdateTodoListTool.execute() normalizedTodos after re-preserve", { + normalizedCount: normalizedTodos.length, + todosWithSubtaskIdCount: normalizedTodos.filter((t) => typeof t?.subtaskId === "string").length, + subtaskIds: normalizedTodos.map((t) => t?.subtaskId).filter(Boolean), + }) + } task.say( "user_edit_todos", @@ -116,15 +309,29 @@ export class UpdateTodoListTool extends BaseTool<"update_todo_list"> { ) } + console.log("[TODO-DEBUG] execute() STEP 18: setting todoList on task", { + finalTodosCount: normalizedTodos.length, + }) await setTodoListForTask(task, normalizedTodos) + console.log("[TODO-DEBUG] execute() STEP 19: setTodoListForTask completed", { + taskTodoListCount: Array.isArray(task?.todoList) ? task.todoList.length : undefined, + }) if (isTodoListChanged) { const md = todoListToMarkdown(normalizedTodos) + console.log("[TODO-DEBUG] execute() STEP 20: returning tool result (user edits)", { + mdLength: md.length, + }) pushToolResult(formatResponse.toolResult("User edits todo:\n\n" + md)) } else { + console.log("[TODO-DEBUG] execute() STEP 20: returning tool result (no user edits)", {}) pushToolResult(formatResponse.toolResult("Todo list updated successfully.")) } } catch (error) { + console.log("[TODO-DEBUG] execute() STEP 99: caught error", { + error: + error instanceof Error ? { name: error.name, message: error.message, stack: error.stack } : error, + }) await handleError("update todo list", error as Error) } } @@ -142,7 +349,8 @@ export class UpdateTodoListTool extends BaseTool<"update_todo_list"> { todos = [] } - todos = preserveTodoMetadata(todos, previousTodos) + // Avoid log spam: partial updates can stream frequently. + todos = preserveTodoMetadata(todos, previousTodos, { enableInstrumentation: false }) const approvalMsg = JSON.stringify({ tool: "updateTodoList", @@ -237,10 +445,56 @@ function normalizeStatus(status: string | undefined): TodoStatus { * 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[] { +function preserveTodoMetadata( + nextTodos: TodoItem[], + previousTodos: TodoItem[], + options?: { enableInstrumentation?: boolean }, +): TodoItem[] { + console.log("[TODO-DEBUG] preserveTodoMetadata() STEP 0: ENTERED", { + nextTodosCount: Array.isArray(nextTodos) ? nextTodos.length : 0, + previousTodosCount: Array.isArray(previousTodos) ? previousTodos.length : 0, + enableInstrumentationOption: options?.enableInstrumentation ?? false, + ROO_DEBUG_TODO_METADATA: process.env.ROO_DEBUG_TODO_METADATA, + ROO_CLI_DEBUG_LOG: process.env.ROO_CLI_DEBUG_LOG, + }) + + const shouldTodoDebugLogToConsole = + process.env.ROO_DEBUG_TODO_METADATA === "1" || + process.env.ROO_DEBUG_TODO_METADATA === "true" || + process.env.ROO_CLI_DEBUG_LOG === "1" + const safePrevious = previousTodos ?? [] const safeNext = nextTodos ?? [] + const summarizeTodoForDebug = (t: TodoItem | undefined) => { + if (!t) return undefined + return { + id: typeof t.id === "string" ? t.id : undefined, + status: typeof t.status === "string" ? t.status : undefined, + content: typeof t.content === "string" ? t.content.substring(0, 50) : undefined, + subtaskId: typeof t.subtaskId === "string" ? t.subtaskId : undefined, + tokens: typeof t.tokens === "number" ? t.tokens : undefined, + cost: typeof t.cost === "number" ? t.cost : undefined, + added: typeof t.added === "number" ? t.added : undefined, + removed: typeof t.removed === "number" ? t.removed : undefined, + } + } + + if (shouldTodoDebugLogToConsole) { + console.log("[TODO-DEBUG] preserveTodoMetadata INPUT", { + previousTodosCount: safePrevious.length, + previousTodos: safePrevious.map((t) => summarizeTodoForDebug(t)), + newTodosCount: safeNext.length, + newTodos: safeNext.map((t) => summarizeTodoForDebug(t)), + }) + } + + // Instrumentation must never write to stdout/stderr (CLI TUI) and should be opt-in. + // Gate it behind an env var so we don't write files during normal operation. + const enableInstrumentation = + (options?.enableInstrumentation ?? false) && + (process.env.ROO_CLI_DEBUG_LOG === "1" || process.env.ROO_DEBUG_TODO_METADATA === "1") + // Track which previous todos have been used (by their index) to avoid double-matching const usedPreviousIndices = new Set() @@ -252,10 +506,21 @@ function preserveTodoMetadata(nextTodos: TodoItem[], previousTodos: TodoItem[]): const previousById = new Map() const previousByContent = new Map>() + const previousMetadataIndices = new Set() for (let i = 0; i < safePrevious.length; i++) { const prev = safePrevious[i] if (!prev) continue + const hasMetadata = + prev.subtaskId !== undefined || + prev.tokens !== undefined || + prev.cost !== undefined || + prev.added !== undefined || + prev.removed !== undefined + if (hasMetadata) { + previousMetadataIndices.add(i) + } + if (typeof prev.subtaskId === "string") { const list = previousBySubtaskId.get(prev.subtaskId) if (list) list.push({ todo: prev, index: i }) @@ -267,26 +532,62 @@ function preserveTodoMetadata(nextTodos: TodoItem[], previousTodos: TodoItem[]): } if (typeof prev.content === "string") { - const list = previousByContent.get(prev.content) + const normalizedContent = normalizeTodoContentForId(prev.content) + const list = previousByContent.get(normalizedContent) if (list) list.push({ todo: prev, index: i }) - else previousByContent.set(prev.content, [{ todo: prev, index: i }]) + else previousByContent.set(normalizedContent, [{ todo: prev, index: i }]) } } - return safeNext.map((next) => { - if (!next) return next + // 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 + } + + if (shouldTodoDebugLogToConsole) { + console.log("[TODO-DEBUG] preserveTodoMetadata ITERATION", { + nextIndex, + next: summarizeTodoForDebug(next), + usedPreviousIndicesCount: usedPreviousIndices.size, + }) + } 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) { + if (shouldTodoDebugLogToConsole) { + console.log("[TODO-DEBUG] preserveTodoMetadata subtaskId candidates", { + nextIndex, + subtaskId: next.subtaskId, + candidatesCount: candidates.length, + }) + } for (const candidate of candidates) { + if (shouldTodoDebugLogToConsole) { + console.log("[TODO-DEBUG] preserveTodoMetadata subtaskId candidate", { + nextIndex, + candidateIndex: candidate.index, + candidate: summarizeTodoForDebug(candidate.todo), + candidateAlreadyUsed: usedPreviousIndices.has(candidate.index), + }) + } if (!usedPreviousIndices.has(candidate.index)) { matchedPrev = candidate.todo matchedIndex = candidate.index + matchStrategy = "subtaskId" break } } @@ -299,18 +600,36 @@ function preserveTodoMetadata(nextTodos: TodoItem[], previousTodos: TodoItem[]): 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 candidates = previousByContent.get(next.content) + const normalizedContent = normalizeTodoContentForId(next.content) + const candidates = previousByContent.get(normalizedContent) if (candidates) { + if (shouldTodoDebugLogToConsole) { + console.log("[TODO-DEBUG] preserveTodoMetadata content candidates", { + nextIndex, + normalizedContent: normalizedContent.substring(0, 50), + candidatesCount: candidates.length, + }) + } // Find first unused candidate for (const candidate of candidates) { + if (shouldTodoDebugLogToConsole) { + console.log("[TODO-DEBUG] preserveTodoMetadata content candidate", { + nextIndex, + candidateIndex: candidate.index, + candidate: summarizeTodoForDebug(candidate.todo), + candidateAlreadyUsed: usedPreviousIndices.has(candidate.index), + }) + } if (!usedPreviousIndices.has(candidate.index)) { matchedPrev = candidate.todo matchedIndex = candidate.index + matchStrategy = "content" break } } @@ -319,8 +638,26 @@ function preserveTodoMetadata(nextTodos: TodoItem[], previousTodos: TodoItem[]): // Mark as used and apply metadata if (matchedPrev && matchedIndex !== undefined) { + const metadataCopiedFromPrev = { + subtaskId: next.subtaskId === undefined ? matchedPrev.subtaskId : undefined, + tokens: next.tokens === undefined ? matchedPrev.tokens : undefined, + cost: next.cost === undefined ? matchedPrev.cost : undefined, + added: next.added === undefined ? matchedPrev.added : undefined, + removed: next.removed === undefined ? matchedPrev.removed : undefined, + } + if (shouldTodoDebugLogToConsole) { + console.log("[TODO-DEBUG] preserveTodoMetadata MATCH", { + nextIndex, + matchStrategy, + matchedIndex, + matchedPrev: summarizeTodoForDebug(matchedPrev), + metadataCopiedFromPrev, + }) + } + usedPreviousIndices.add(matchedIndex) - return { + matchedPreviousIndexByNextIndex[nextIndex] = matchedIndex + result[nextIndex] = { ...next, subtaskId: next.subtaskId ?? matchedPrev.subtaskId, tokens: next.tokens ?? matchedPrev.tokens, @@ -328,10 +665,191 @@ function preserveTodoMetadata(nextTodos: TodoItem[], previousTodos: TodoItem[]): added: next.added ?? matchedPrev.added, removed: next.removed ?? matchedPrev.removed, } + continue } - return next - }) + if (shouldTodoDebugLogToConsole) { + console.log("[TODO-DEBUG] preserveTodoMetadata NO_MATCH", { + nextIndex, + next: summarizeTodoForDebug(next), + }) + } + + 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) { + let indexCarryoverCount = 0 + 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 + + if (shouldTodoDebugLogToConsole) { + console.log("[TODO-DEBUG] preserveTodoMetadata INDEX_CARRYOVER", { + nextIndex: i, + previousIndex: i, + prev: summarizeTodoForDebug(prev), + next: summarizeTodoForDebug(next), + metadataCopiedFromPrev: { + subtaskId: next.subtaskId === undefined ? prev.subtaskId : undefined, + tokens: next.tokens === undefined ? prev.tokens : undefined, + cost: next.cost === undefined ? prev.cost : undefined, + added: next.added === undefined ? prev.added : undefined, + removed: next.removed === undefined ? prev.removed : undefined, + }, + }) + } + + result[i] = { + ...next, + subtaskId: next.subtaskId ?? prev.subtaskId, + tokens: next.tokens ?? prev.tokens, + cost: next.cost ?? prev.cost, + added: next.added ?? prev.added, + removed: next.removed ?? prev.removed, + } + usedPreviousIndices.add(i) + indexCarryoverCount++ + } + + if (enableInstrumentation && indexCarryoverCount > 0) { + appendRooCliDebugLog("[Roo-Debug] preserveTodoMetadata: applied index-based rename carryover", { + indexCarryoverCount, + previousTodosCount: safePrevious.length, + nextTodosCount: safeNext.length, + }) + } + } + + // 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, + added: targetNext.added ?? orphanedPrev.added, + removed: targetNext.removed ?? orphanedPrev.removed, + } + + result[targetNextIndex] = updatedTarget + usedPreviousIndices.add(orphanedPrevIndex) + matchedPreviousIndexByNextIndex[targetNextIndex] = orphanedPrevIndex + + if (shouldTodoDebugLogToConsole) { + console.log("[TODO-DEBUG] preserveTodoMetadata ORPHAN_CARRYOVER", { + orphanedContent: orphanedPrev.content?.substring(0, 40), + orphanedSubtaskId: orphanedPrev.subtaskId, + targetContent: updatedTarget.content?.substring(0, 40), + copiedFields: { + subtaskId: updatedTarget.subtaskId, + tokens: updatedTarget.tokens, + cost: updatedTarget.cost, + }, + }) + } + } + + // Lightweight debug instrumentation: detect when previous rows that had metadata could not be + // matched to any next todo row (and therefore their metadata could not be preserved). + // + // Keep payload minimal to avoid logging user content. + if (enableInstrumentation && previousMetadataIndices.size > 0) { + let lostMetadataRowCount = 0 + for (const prevIndex of previousMetadataIndices) { + if (!usedPreviousIndices.has(prevIndex)) { + lostMetadataRowCount++ + } + } + + if (lostMetadataRowCount > 0) { + if (shouldTodoDebugLogToConsole) { + console.log("[TODO-DEBUG] preserveTodoMetadata LOST_METADATA", { + lostMetadataRowCount, + previousMetadataRowCount: previousMetadataIndices.size, + previousTodosCount: safePrevious.length, + nextTodosCount: safeNext.length, + }) + } + // IMPORTANT: do not use console.log here in the CLI TUI. It can corrupt rendering (e.g. dropdowns). + appendRooCliDebugLog("[Roo-Debug] preserveTodoMetadata: previous todo(s) with metadata were not matched", { + lostMetadataRowCount, + previousMetadataRowCount: previousMetadataIndices.size, + previousTodosCount: safePrevious.length, + nextTodosCount: safeNext.length, + }) + } + } + + if (shouldTodoDebugLogToConsole) { + console.log("[TODO-DEBUG] preserveTodoMetadata OUTPUT", { + resultCount: result.length, + resultTodos: result.map((t) => summarizeTodoForDebug(t)), + }) + } + + 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 +} + +const ROO_CLI_DEBUG_LOG_PATH = path.join(os.tmpdir(), "roo-cli-debug.log") + +function appendRooCliDebugLog(message: string, data?: unknown) { + try { + const timestamp = new Date().toISOString() + const entry = data ? `[${timestamp}] ${message}: ${JSON.stringify(data)}\n` : `[${timestamp}] ${message}\n` + fs.appendFileSync(ROO_CLI_DEBUG_LOG_PATH, entry) + } catch { + // Swallow errors: logging must never break tool execution. + } } export function parseMarkdownChecklist(md: string): TodoItem[] { @@ -340,6 +858,11 @@ export function parseMarkdownChecklist(md: string): TodoItem[] { .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+(.+)$/) @@ -347,19 +870,84 @@ 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 } : {}), + ...(typeof t.added === "number" ? { added: t.added } : {}), + ...(typeof t.removed === "number" ? { removed: t.removed } : {}), + } + + 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 c67730e6c8..af96c352f7 100644 --- a/src/core/tools/__tests__/updateTodoListTool.spec.ts +++ b/src/core/tools/__tests__/updateTodoListTool.spec.ts @@ -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,6 +247,14 @@ 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) + }) }) }) @@ -247,6 +263,244 @@ describe("UpdateTodoListTool.execute", () => { setPendingTodoList([]) }) + it("should preserve per-row metadata (subtaskId/tokens/cost/added/removed) 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), + added: 10 * (idx + 1), + removed: idx, + })) + + 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, + added: 10, + removed: 0, + }), + ) + + expect(task.todoList[1]).toEqual( + expect.objectContaining({ + content: "Task B", + status: "completed", + subtaskId: "subtask-2", + tokens: 1001, + cost: 0.02, + added: 20, + removed: 1, + }), + ) + + expect(task.todoList[2]).toEqual( + expect.objectContaining({ + content: "Task A", + status: "completed", + subtaskId: "subtask-3", + tokens: 1002, + cost: 0.03, + added: 30, + removed: 2, + }), + ) + + expect(task.todoList[3]).toEqual( + expect.objectContaining({ + content: "Task C", + status: "pending", + subtaskId: "subtask-4", + tokens: 1003, + cost: 0.04, + added: 40, + removed: 3, + }), + ) + }) + + 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), + added: 10 * (idx + 1), + removed: idx, + })) + + 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, + added: 10, + removed: 0, + }), + ) + expect(task.todoList[1]).toEqual( + expect.objectContaining({ + content: "New B", + status: "completed", + subtaskId: "subtask-2", + tokens: 101, + cost: 0.02, + added: 20, + removed: 1, + }), + ) + expect(task.todoList[2]).toEqual( + expect.objectContaining({ + content: "New C", + status: "in_progress", + subtaskId: "subtask-3", + tokens: 102, + cost: 0.03, + added: 30, + removed: 2, + }), + ) + }) + + 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, + added: 11, + removed: 1, + }, + { + id: "id-2", + content: "Beta", + status: "completed", + subtaskId: "subtask-2", + tokens: 222, + cost: 0.22, + added: 22, + removed: 2, + }, + ] + + // 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, + added: 22, + removed: 2, + }), + ) + + expect(task.todoList[1]).toEqual( + expect.objectContaining({ + id: "id-1", + content: "Alpha renamed", + status: "pending", + subtaskId: "subtask-1", + tokens: 111, + cost: 0.11, + added: 11, + removed: 1, + }), + ) + }) + it("should prefer history todos when they contain metadata (subtaskId/tokens/cost)", async () => { const md = "[ ] Task 1" @@ -436,6 +690,223 @@ describe("UpdateTodoListTool.execute", () => { ) }) + 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, + added: 11, + removed: 1, + }, + { + id: "legacy-2", + content: "Task 2", + status: "pending", + tokens: 222, + cost: 0.22, + added: 22, + removed: 2, + }, + ] + + 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, + added: 11, + removed: 1, + }), + ) + expect(task2).toEqual( + expect.objectContaining({ + content: "Task 2", + status: "pending", + tokens: 222, + cost: 0.22, + added: 22, + removed: 2, + }), + ) + }) + + 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, + added: 10, + removed: 2, + }, + ] + + // 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, + added: 10, + removed: 2, + }), + ) + + // 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() + expect(task.todoList[1].added).toBeUndefined() + expect(task.todoList[1].removed).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, + added: 10, + removed: 2, + }, + { + 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, + added: 10, + removed: 2, + }), + ) + // 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, + added: 10, + removed: 2, + }), + ) + + // 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 diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index dd74a259db..15f321451a 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -1694,18 +1694,43 @@ 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) // 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, async (id: string) => { - const result = await this.getTaskWithId(id) - return result.historyItem - }) + childDetails = await buildSubtaskDetails(aggregatedCosts.childBreakdown, loadTaskHistoryItemForAggregation) } return { @@ -2558,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 @@ -3174,6 +3201,16 @@ export class ClineProvider 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 = [] @@ -3215,6 +3252,17 @@ export class ClineProvider } // 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, @@ -3337,6 +3385,18 @@ export class ClineProvider // 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 = [] } @@ -3344,6 +3404,7 @@ export class ClineProvider 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. @@ -3364,15 +3425,35 @@ export class ClineProvider // 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 linkedTodo.added = childHistoryItem?.linesAdded || 0 linkedTodo.removed = childHistoryItem?.linesRemoved || 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", 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 0000000000..f7d00d435b --- /dev/null +++ b/src/core/webview/__tests__/ClineProvider.reopenParentFromDelegation.writeback.spec.ts @@ -0,0 +1,237 @@ +// 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, + linesAdded: 7, + linesRemoved: 2, + } 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, + added: 7, + removed: 2, + }), + ) + + // 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(updatedChild1Row?.added).toBeUndefined() + expect(updatedChild1Row?.removed).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 000460471a..c11741af15 100644 --- a/src/core/webview/__tests__/aggregateTaskCosts.spec.ts +++ b/src/core/webview/__tests__/aggregateTaskCosts.spec.ts @@ -233,6 +233,74 @@ 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, + linesAdded: 1, + linesRemoved: 0, + childIds: ["child-ok", "child-missing"], + } as unknown as HistoryItem, + "child-ok": { + id: "child-ok", + totalCost: 0.5, + linesAdded: 3, + linesRemoved: 2, + 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) + expect(result.childrenAdded).toBe(3) + expect(result.childrenRemoved).toBe(2) + expect(result.totalAdded).toBe(4) + expect(result.totalRemoved).toBe(2) + + const childOk = result.childBreakdown?.["child-ok"] + expect(childOk).toBeDefined() + expect(childOk!.totalCost).toBe(0.5) + expect(childOk!.totalAdded).toBe(3) + expect(childOk!.totalRemoved).toBe(2) + + // 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 = {} diff --git a/src/shared/todo.ts b/src/shared/todo.ts index 81e4559a93..3f2ca0c3fa 100644 --- a/src/shared/todo.ts +++ b/src/shared/todo.ts @@ -1,26 +1,51 @@ -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" || msg.say === "system_update_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) { + console.log("[TODO-DEBUG]", "getLatestTodo called with empty clineMessages") 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[] + } + } + + console.log("[TODO-DEBUG]", "getLatestTodo scanned messages", { + totalMessages: clineMessages.length, + candidateMessages: candidateMessages.length, + matchedUpdateTodoListCount, + parseFailureCount, + returnedTodosCount: Array.isArray(lastTodos) ? lastTodos.length : 0, + // Only log lightweight metadata for the last few candidates (avoid dumping full message content) + lastCandidates: candidateMessages.slice(-5).map((m) => ({ + ts: m.ts, + type: m.type, + ask: (m as any).ask, + say: (m as any).say, + textLength: typeof m.text === "string" ? m.text.length : 0, + })), + }) + + return Array.isArray(lastTodos) ? lastTodos : [] } diff --git a/webview-ui/src/components/chat/ChatRow.tsx b/webview-ui/src/components/chat/ChatRow.tsx index 89d817e4aa..3a88223190 100644 --- a/webview-ui/src/components/chat/ChatRow.tsx +++ b/webview-ui/src/components/chat/ChatRow.tsx @@ -393,10 +393,22 @@ 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) + + // TODO debugging: verify tool JSON is actually being parsed in the webview. + if ((parsed as any)?.tool === "updateTodoList") { + console.log("[TODO-DEBUG]", "ChatRow parsed tool JSON", { + messageTs: message.ts, + toolName: (parsed as any)?.tool, + newTodosCount: Array.isArray((parsed as any)?.todos) ? (parsed as any).todos.length : undefined, + parsed, + }) + } + + return parsed + }, [message.ask, message.text, message.ts]) // Unified diff content (provided by backend when relevant) const unifiedDiff = useMemo(() => { @@ -569,6 +581,15 @@ export const ChatRowContent = ({ // Get previous todos from the latest todos in the task context const previousTodos = getPreviousTodos(clineMessages, message.ts) + console.log("[TODO-DEBUG]", "ChatRow rendering TodoChangeDisplay", { + messageTs: message.ts, + previousTodosCount: Array.isArray(previousTodos) ? previousTodos.length : undefined, + newTodosCount: Array.isArray(todos) ? todos.length : undefined, + previousTodos, + newTodos: todos, + parsedTool: tool, + }) + return } case "newFileCreated": diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index d222c8c504..17ec30d70f 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -125,6 +125,16 @@ const ChatViewComponent: React.ForwardRefRenderFunction { + // TODO debugging: ensure todo extraction runs and surfaces state that should drive UI. + console.log("[TODO-DEBUG]", "ChatView latestTodos computed", { + messagesCount: Array.isArray(messages) ? messages.length : undefined, + currentTaskTodosCount: Array.isArray(currentTaskTodos) ? currentTaskTodos.length : undefined, + latestTodosCount: Array.isArray(latestTodos) ? latestTodos.length : undefined, + latestTodos, + }) + }, [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. diff --git a/webview-ui/src/components/chat/TodoChangeDisplay.tsx b/webview-ui/src/components/chat/TodoChangeDisplay.tsx index 3904cd9c9f..d2043f435b 100644 --- a/webview-ui/src/components/chat/TodoChangeDisplay.tsx +++ b/webview-ui/src/components/chat/TodoChangeDisplay.tsx @@ -26,6 +26,17 @@ function getTodoIcon(status: TodoStatus | null) { } export function TodoChangeDisplay({ previousTodos, newTodos }: TodoChangeDisplayProps) { + console.log("[TODO-DEBUG]", "TodoChangeDisplay compare todos", { + previousTodosCount: Array.isArray(previousTodos) ? previousTodos.length : 0, + newTodosCount: Array.isArray(newTodos) ? newTodos.length : 0, + previousTodos: Array.isArray(previousTodos) + ? previousTodos.map((t) => ({ id: t.id, content: t.content, status: t.status })) + : [], + newTodos: Array.isArray(newTodos) + ? newTodos.map((t) => ({ id: t.id, content: t.content, status: t.status })) + : [], + }) + const isInitialState = previousTodos.length === 0 // Determine which todos to display @@ -34,19 +45,45 @@ export function TodoChangeDisplay({ previousTodos, newTodos }: TodoChangeDisplay if (isInitialState && newTodos.length > 0) { // For initial state, show all todos in their original order todosToDisplay = newTodos + console.log("[TODO-DEBUG]", "TodoChangeDisplay selection: initial state -> show all newTodos", { + todosToDisplayCount: todosToDisplay.length, + }) } else { // For updates, only show changes (completed or started) in their original order 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" + console.log("[TODO-DEBUG]", "TodoChangeDisplay selection: completed todo", { + newTodo: { id: newTodo.id, content: newTodo.content, status: newTodo.status }, + matchedPreviousTodo: previousTodo + ? { id: previousTodo.id, content: previousTodo.content, status: previousTodo.status } + : undefined, + include, + }) + 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" + console.log("[TODO-DEBUG]", "TodoChangeDisplay selection: in_progress todo", { + newTodo: { id: newTodo.id, content: newTodo.content, status: newTodo.status }, + matchedPreviousTodo: previousTodo + ? { id: previousTodo.id, content: previousTodo.content, status: previousTodo.status } + : undefined, + include, + }) + return include } + console.log("[TODO-DEBUG]", "TodoChangeDisplay selection: ignored todo (not completed/in_progress)", { + newTodo: { id: newTodo.id, content: newTodo.content, status: newTodo.status }, + }) return false }) + console.log("[TODO-DEBUG]", "TodoChangeDisplay selection result", { + todosToDisplayCount: todosToDisplay.length, + todosToDisplay: todosToDisplay.map((t) => ({ id: t.id, content: t.content, status: t.status })), + }) } // If no todos to display, don't render anything diff --git a/webview-ui/src/components/chat/TodoListDisplay.tsx b/webview-ui/src/components/chat/TodoListDisplay.tsx index c3da1c717b..902206d66f 100644 --- a/webview-ui/src/components/chat/TodoListDisplay.tsx +++ b/webview-ui/src/components/chat/TodoListDisplay.tsx @@ -41,6 +41,16 @@ export interface TodoListDisplayProps { } export function TodoListDisplay({ todos, subtaskDetails, onSubtaskClick }: TodoListDisplayProps) { + useEffect(() => { + console.log("[TODO-DEBUG]", "TodoListDisplay props received", { + todosCount: Array.isArray(todos) ? todos.length : 0, + todoSubtaskIds: Array.isArray(todos) ? todos.map((t) => t?.subtaskId).filter(Boolean) : [], + subtaskDetailsCount: Array.isArray(subtaskDetails) ? subtaskDetails.length : 0, + subtaskDetailsIds: Array.isArray(subtaskDetails) ? subtaskDetails.map((s) => s?.id).filter(Boolean) : [], + hasOnSubtaskClick: Boolean(onSubtaskClick), + }) + }, [todos, subtaskDetails, onSubtaskClick]) + const [isCollapsed, setIsCollapsed] = useState(true) const ulRef = useRef(null) const itemRefs = useRef<(HTMLLIElement | null)[]>([]) @@ -108,10 +118,25 @@ export function TodoListDisplay({ todos, subtaskDetails, onSubtaskClick }: TodoL const todoStatus = (todo.status as TodoStatus) ?? "pending" const icon = getTodoIcon(todoStatus) const isClickable = Boolean(todo.subtaskId && onSubtaskClick) + console.log("[TODO-DEBUG]", "TodoListDisplay subtask match start", { + todoIndex: idx, + todoId: todo.id, + todoContent: todo.content, + todoSubtaskId: todo.subtaskId, + availableSubtaskDetailIds: Array.isArray(subtaskDetails) + ? subtaskDetails.map((s) => s?.id).filter(Boolean) + : [], + }) const subtaskById = subtaskDetails && todo.subtaskId ? subtaskDetails.find((s) => s.id === todo.subtaskId) : undefined + console.log("[TODO-DEBUG]", "TodoListDisplay subtask match result", { + todoIndex: idx, + todoSubtaskId: todo.subtaskId, + matched: Boolean(subtaskById), + matchedSubtaskId: subtaskById?.id, + }) const displayTokens = todo.tokens ?? subtaskById?.tokens const displayCost = todo.cost ?? subtaskById?.cost const shouldShowCost = typeof displayTokens === "number" && typeof displayCost === "number" @@ -139,6 +164,33 @@ export function TodoListDisplay({ todos, subtaskDetails, onSubtaskClick }: TodoL const shouldShowLineChanges = hasValidSubtaskLink && (canRenderAdded || canRenderRemoved) + console.log("[TODO-DEBUG]", "TodoListDisplay metadata computed", { + todoIndex: idx, + todoSubtaskId: todo.subtaskId, + fromTodo: { + tokens: todo.tokens, + cost: todo.cost, + added: todo.added, + removed: todo.removed, + }, + fromSubtaskDetails: subtaskById + ? { + tokens: subtaskById.tokens, + cost: subtaskById.cost, + added: subtaskById.added, + removed: subtaskById.removed, + } + : undefined, + display: { + displayTokens, + displayCost, + displayAdded, + displayRemoved, + }, + shouldShowCost, + shouldShowLineChanges, + }) + const isAddedPositive = canRenderAdded && (displayAdded as number) > 0 const isRemovedPositive = canRenderRemoved && (displayRemoved as number) > 0 const isAddedZero = canRenderAdded && displayAdded === 0