fix: improve todo-subtask linking reliability with fallback anchors

Enhance the direct todo-subtask linking mechanism to handle edge cases
where subtaskId links may be missing during history resume or delegation.

Changes:
- UpdateTodoListTool: Use dual-strategy metadata matching (ID-first,
  then content-based fallback) to preserve tokens/cost across updates
- ClineProvider: Add deterministic fallback anchor selection
  (in_progress > pending > last completed > synthetic) for both
  delegation-time linking and cost write-back on resume
- ClineProvider: Create synthetic anchor todo when no todos exist
- Clean up excessive logging statements

Tests:
- Add fallback anchor tests for reopenParentFromDelegation
- Add delegation-time linking tests with various todo states
This commit is contained in:
Toray Altas 2026-01-17 15:47:52 -05:00
parent 6b3ec95ce3
commit 4c9dd28d40
4 changed files with 415 additions and 96 deletions

View file

@ -491,4 +491,175 @@ 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: "user_edit_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 user_edit_todos message that was added for the write-back
const todoEditMessages = savedCall.messages.filter((m: any) => m.type === "say" && m.say === "user_edit_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: "user_edit_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 === "user_edit_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()
}
})
})

View file

@ -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")
})
})

View file

@ -202,27 +202,94 @@ 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. **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.
* 2. **Content match with position awareness**: For todos without matching 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 changes (which can alter the derived ID)
* and handles duplicates deterministically.
*/
function preserveTodoMetadata(nextTodos: TodoItem[], previousTodos: TodoItem[]): TodoItem[] {
// Build content -> queue mapping so duplicates are matched in order.
const previousByContent = new Map<string, TodoItem[]>()
for (const prev of previousTodos ?? []) {
if (!prev || typeof prev.content !== "string") continue
const list = previousByContent.get(prev.content)
if (list) list.push(prev)
else previousByContent.set(prev.content, [prev])
const safePrevious = previousTodos ?? []
const safeNext = nextTodos ?? []
// Build ID -> todo mapping for O(1) lookup
const previousById = new Map<string, TodoItem>()
for (const prev of safePrevious) {
if (prev?.id && typeof prev.id === "string") {
// Only store the first occurrence for each ID (handle duplicates deterministically)
if (!previousById.has(prev.id)) {
previousById.set(prev.id, prev)
}
}
}
return (nextTodos ?? []).map((next) => {
const candidates = previousByContent.get(next.content)
const matchedPrev = candidates?.shift()
if (!matchedPrev) return next
// Track which previous todos have been used (by their index) to avoid double-matching
const usedPreviousIndices = new Set<number>()
return {
...next,
subtaskId: next.subtaskId ?? matchedPrev.subtaskId,
tokens: next.tokens ?? matchedPrev.tokens,
cost: next.cost ?? matchedPrev.cost,
// Build content -> queue mapping for fallback (content-based matching)
// Each queue entry includes the original index for tracking
const previousByContent = new Map<string, Array<{ todo: TodoItem; index: number }>>()
for (let i = 0; i < safePrevious.length; i++) {
const prev = safePrevious[i]
if (!prev || typeof prev.content !== "string") continue
const list = previousByContent.get(prev.content)
if (list) list.push({ todo: prev, index: i })
else previousByContent.set(prev.content, [{ todo: prev, index: i }])
}
return safeNext.map((next) => {
if (!next) return next
let matchedPrev: TodoItem | undefined = undefined
let matchedIndex: number | undefined = undefined
// Strategy 1: Try ID-based matching first (most reliable)
if (next.id && typeof next.id === "string") {
const byId = previousById.get(next.id)
if (byId) {
// Find the index of this todo in the original array
const idx = safePrevious.findIndex((p) => p === byId)
if (idx !== -1 && !usedPreviousIndices.has(idx)) {
matchedPrev = byId
matchedIndex = idx
}
}
}
// 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)
if (candidates) {
// Find first unused candidate
for (const candidate of candidates) {
if (!usedPreviousIndices.has(candidate.index)) {
matchedPrev = candidate.todo
matchedIndex = candidate.index
break
}
}
}
}
// Mark as used and apply metadata
if (matchedPrev && matchedIndex !== undefined) {
usedPreviousIndices.add(matchedIndex)
return {
...next,
subtaskId: next.subtaskId ?? matchedPrev.subtaskId,
tokens: next.tokens ?? matchedPrev.tokens,
cost: next.cost ?? matchedPrev.cost,
}
}
return next
})
}

View file

@ -523,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
@ -532,8 +531,6 @@ export class ClineProvider
timeoutId,
createdAt: Date.now(),
})
this.log(`[setPendingEditOperation] Set pending operation: ${operationId}`)
}
/**
@ -551,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
@ -565,7 +561,6 @@ export class ClineProvider
clearTimeout(operation.timeoutId)
}
this.pendingOperations.clear()
this.log(`[clearAllPendingEditOperations] Cleared all pending operations`)
}
/*
@ -583,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()
@ -624,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
@ -844,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
@ -1032,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
@ -1050,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 {
@ -2887,10 +2863,6 @@ export class ClineProvider
await this.addClineToStack(task)
this.log(
`[createTask] ${task.parentTask ? "child" : "parent"} task ${task.taskId}.${task.instanceId} instantiated`,
)
return task
}
@ -2901,8 +2873,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.
@ -2969,8 +2939,6 @@ export class ClineProvider
// This is used when the user cancels a task that is not a subtask.
public async clearTask(): Promise<void> {
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()
}
}
@ -3202,59 +3170,69 @@ export class ClineProvider
// 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 })
const todos = getLatestTodo(parentMessages) as unknown as TodoItem[]
let todos = (getLatestTodo(parentMessages) as unknown as TodoItem[]) ?? []
// 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")
// Deterministic selection rule (in_progress > pending): pick the first matching item
// in the list order, even if multiple candidates exist.
const chosen: TodoItem | undefined = inProgress[0] ?? pending[0]
if (!chosen) {
this.log(
`[delegateParentAndOpenChild] Not linking subtask ${child.taskId}: no in_progress or pending todos found`,
)
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 {
// Log ambiguity (but still link deterministically).
if (inProgress.length > 1) {
this.log(
`[delegateParentAndOpenChild] Multiple in_progress todos (${inProgress.length}); linking first to subtask ${child.taskId}`,
)
} else if (pending.length > 1 && inProgress.length === 0) {
this.log(
`[delegateParentAndOpenChild] Multiple pending todos (${pending.length}); linking first to subtask ${child.taskId}`,
)
// 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
}
if (chosen) {
if (chosen.subtaskId && chosen.subtaskId !== child.taskId) {
this.log(
`[delegateParentAndOpenChild] Overwriting existing todo.subtaskId '${chosen.subtaskId}' -> '${child.taskId}'`,
)
}
// Set the subtaskId on the chosen todo (unless it's the synthetic one we just created)
if (chosen && !chosen.subtaskId) {
chosen.subtaskId = child.taskId
await saveTaskMessages({
messages: [
...parentMessages,
{
ts: Date.now(),
type: "say",
say: "user_edit_todos",
text: JSON.stringify({
tool: "updateTodoList",
todos,
}),
},
],
taskId: parentTaskId,
globalStoragePath,
})
}
// Always persist the updated todo list
await saveTaskMessages({
messages: [
...parentMessages,
{
ts: Date.now(),
type: "say",
say: "user_edit_todos",
text: JSON.stringify({
tool: "updateTodoList",
todos,
}),
},
],
taskId: parentTaskId,
globalStoragePath,
})
} catch (error) {
this.log(
`[delegateParentAndOpenChild] Failed to persist delegation-time todo link (non-fatal): ${
@ -3356,15 +3334,42 @@ export class ClineProvider
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 {
const todos = getLatestTodo(parentClineMessages) as unknown as TodoItem[]
if (Array.isArray(todos) && todos.length > 0) {
const linkedTodo = todos.find((t) => t?.subtaskId === childTaskId)
if (!linkedTodo) {
this.log(
`[reopenParentFromDelegation] No todo found with subtaskId === ${childTaskId}; skipping cost write-back`,
)
} else {
let todos = (getLatestTodo(parentClineMessages) as unknown as TodoItem[]) ?? []
if (!Array.isArray(todos)) {
todos = []
}
if (todos.length > 0) {
// Primary lookup by subtaskId
let linkedTodo = todos.find((t) => t?.subtaskId === childTaskId)
// 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
}
}
if (linkedTodo) {
linkedTodo.tokens = (childHistoryItem?.tokensIn || 0) + (childHistoryItem?.tokensOut || 0)
linkedTodo.cost = childHistoryItem?.totalCost || 0