From 188a1c6e8ebd897f272ecda11702aacf9aec33da Mon Sep 17 00:00:00 2001 From: Toray Altas Date: Sun, 18 Jan 2026 00:31:41 -0500 Subject: [PATCH] fix: preserve todo metadata on updateTodoList Prefer history todos when they contain subtaskId/tokens/cost so updateTodoList preserves injected metadata. Adds regression test covering history-vs-memory selection. --- src/core/tools/UpdateTodoListTool.ts | 22 ++++++-- .../__tests__/updateTodoListTool.spec.ts | 54 ++++++++++++++++++- 2 files changed, 70 insertions(+), 6 deletions(-) diff --git a/src/core/tools/UpdateTodoListTool.ts b/src/core/tools/UpdateTodoListTool.ts index 068d473b53..a895f08ca7 100644 --- a/src/core/tools/UpdateTodoListTool.ts +++ b/src/core/tools/UpdateTodoListTool.ts @@ -26,11 +26,23 @@ export class UpdateTodoListTool extends BaseTool<"update_todo_list"> { const { pushToolResult, handleError, askApproval, toolProtocol } = callbacks try { - // Pull the previous todo list so we can preserve metadata fields across update_todo_list calls. - // Prefer the in-memory task.todoList when available; otherwise fall back to the latest todo list - // stored in the conversation history. - const previousTodos = - getTodoListForTask(task) ?? (getLatestTodo(task.clineMessages) as unknown as TodoItem[]) + 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 diff --git a/src/core/tools/__tests__/updateTodoListTool.spec.ts b/src/core/tools/__tests__/updateTodoListTool.spec.ts index ebe0500d66..8ed33c10e5 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", () => { @@ -241,3 +241,55 @@ Just some text }) }) }) + +describe("UpdateTodoListTool.execute", () => { + beforeEach(() => { + setPendingTodoList([]) + }) + + 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, + }), + ) + }) +})