From 1060ce1835335cdc0039862e8fab5e882aaaa241 Mon Sep 17 00:00:00 2001 From: Eason Liang Date: Sun, 16 Aug 2026 15:47:04 +0800 Subject: [PATCH] feat(task): track task nesting depth with cycle-safe backfill - Add optional depth (root = 0) to HistoryItem schema, CreateTaskOptions and TaskLike - Derive depth in the Task constructor: persisted value > live parent (+1) > root (0); mark legacy children resumed without their live parent as non-authoritative so a placeholder 0 is never persisted - Persist depth via taskMetadata() only when authoritative - Backfill depth for resumed legacy tasks in createTaskWithHistoryItem by walking the parentTaskId chain through the history store / global state (bounded, cycle-safe) --- packages/types/src/history.ts | 1 + packages/types/src/task.ts | 4 + src/core/task-persistence/taskMetadata.ts | 4 + src/core/task/Task.ts | 33 ++++++++ src/core/task/__tests__/Task.spec.ts | 92 +++++++++++++++++++++++ src/core/task/__tests__/taskDepth.spec.ts | 85 +++++++++++++++++++++ src/core/task/taskDepth.ts | 69 +++++++++++++++++ src/core/webview/ClineProvider.ts | 42 +++++++++++ 8 files changed, 330 insertions(+) create mode 100644 src/core/task/__tests__/taskDepth.spec.ts create mode 100644 src/core/task/taskDepth.ts diff --git a/packages/types/src/history.ts b/packages/types/src/history.ts index 5b173c6a6b..e2a016a450 100644 --- a/packages/types/src/history.ts +++ b/packages/types/src/history.ts @@ -24,6 +24,7 @@ export const historyItemSchema = z.object({ delegatedToId: z.string().optional(), // Last child this parent delegated to childIds: z.array(z.string()).optional(), // All children spawned by this task awaitingChildId: z.string().optional(), // Child currently awaited (set when delegated) + depth: z.number().int().min(0).optional(), // Nesting level; root = 0, child = parent.depth + 1 completedByChildId: z.string().optional(), // Child that completed and resumed this parent completionResultSummary: z.string().optional(), // Summary from completed child }) diff --git a/packages/types/src/task.ts b/packages/types/src/task.ts index 572302861b..b253fc39c4 100644 --- a/packages/types/src/task.ts +++ b/packages/types/src/task.ts @@ -94,6 +94,8 @@ export interface CreateTaskOptions { /** Whether to start the task loop immediately (default: true). * When false, the caller must invoke `task.start()` manually. */ startTask?: boolean + /** Nesting level for newly created tasks; root = 0, child = parent.depth + 1. */ + depth?: number } export enum TaskStatus { @@ -116,6 +118,8 @@ export interface TaskLike { readonly rootTaskId?: string readonly parentTaskId?: string readonly childTaskId?: string + /** Nesting level; root = 0, child = parent.depth + 1. */ + readonly depth: number readonly metadata: TaskMetadata readonly taskStatus: TaskStatus readonly taskAsk: ClineMessage | undefined diff --git a/src/core/task-persistence/taskMetadata.ts b/src/core/task-persistence/taskMetadata.ts index ec2e6cceeb..72a261f946 100644 --- a/src/core/task-persistence/taskMetadata.ts +++ b/src/core/task-persistence/taskMetadata.ts @@ -25,6 +25,8 @@ export type TaskMetadataOptions = { apiConfigName?: string /** Initial status for the task (e.g., "active" for child tasks) */ initialStatus?: "active" | "delegated" | "completed" | "interrupted" + /** Nesting level; root = 0, child = parent.depth + 1. Persisted when known. */ + depth?: number } export async function taskMetadata({ @@ -38,6 +40,7 @@ export async function taskMetadata({ mode, apiConfigName, initialStatus, + depth, }: TaskMetadataOptions) { const taskDir = await getTaskDirectoryPath(globalStoragePath, id) @@ -112,6 +115,7 @@ export async function taskMetadata({ mode, ...(typeof apiConfigName === "string" && apiConfigName.length > 0 ? { apiConfigName } : {}), ...(initialStatus && { status: initialStatus }), + ...(typeof depth === "number" && Number.isInteger(depth) && depth >= 0 ? { depth } : {}), } return { historyItem, tokenUsage } diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index b728e43b9a..cc5f316253 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -170,6 +170,14 @@ export class Task extends EventEmitter implements TaskLike { readonly rootTaskId?: string readonly parentTaskId?: string childTaskId?: string + /** Nesting level; root = 0, child = parent.depth + 1. */ + readonly depth: number + /** + * True when `depth` was derived authoritatively (persisted value, live parent, + * or a genuine root) and is safe to persist on save. False for legacy children + * resumed without their live parent — the provider backfills those before first save. + */ + readonly depthAuthoritative: boolean pendingNewTaskToolCallId?: string readonly instanceId: string @@ -505,6 +513,28 @@ export class Task extends EventEmitter implements TaskLike { this.parentTaskId = historyItem ? historyItem.parentTaskId : parentTask?.taskId this.childTaskId = undefined + // Nesting depth (root = 0). A persisted value is authoritative; otherwise derive it + // from the live parent task (parent.depth + 1) or default to root (0). + const persistedDepth = historyItem?.depth + if (typeof persistedDepth === "number" && Number.isInteger(persistedDepth) && persistedDepth >= 0) { + this.depth = persistedDepth + this.depthAuthoritative = true + } else if (parentTask) { + // Live parent available: its depth is authoritative, so the child's is too. + this.depth = parentTask.depth + 1 + this.depthAuthoritative = true + } else if (!this.parentTaskId) { + // No persisted depth and no parent reference at all: this task is a root. + this.depth = 0 + this.depthAuthoritative = true + } else { + // Legacy child resumed without its live parent (e.g. reopened from history): + // the depth cannot be derived here, so mark it non-authoritative and let + // ClineProvider.createTaskWithHistoryItem() backfill it before first save. + this.depth = 0 + this.depthAuthoritative = false + } + this.metadata = { task: historyItem ? historyItem.task : task, images: historyItem ? [] : images, @@ -1112,6 +1142,9 @@ export class Task extends EventEmitter implements TaskLike { mode: this._taskMode || defaultModeSlug, // Use the task's own mode, not the current provider mode. apiConfigName: this._taskApiConfigName, // Use the task's own provider profile, not the current provider profile. initialStatus: this.initialStatus, + // Only persist depth when it was derived authoritatively; a legacy child resumed + // without its live parent carries a placeholder 0 that must not be written back. + depth: this.depthAuthoritative ? this.depth : undefined, }) // Emit token/tool usage updates using debounced function diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 60bc2f3192..ee2d4d11ba 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -541,6 +541,98 @@ describe("Cline", () => { new Task({ provider: mockProvider, apiConfiguration: mockApiConfig }) }).toThrow("Either historyItem or task/images must be provided") }) + + describe("nesting depth", () => { + it("assigns depth 0 and authoritative to a new root task", () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "root task", + startTask: false, + }) + + expect(task.depth).toBe(0) + expect(task.depthAuthoritative).toBe(true) + }) + + it("derives child depth from the live parent (parent.depth + 1)", () => { + const parent = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "parent task", + startTask: false, + }) + + const child = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "child task", + parentTask: parent, + startTask: false, + }) + + expect(parent.depth).toBe(0) + expect(child.depth).toBe(1) + expect(child.depthAuthoritative).toBe(true) + expect(child.parentTaskId).toBe(parent.taskId) + }) + + it("prefers a persisted depth over the live parent", () => { + const parent = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "parent task", + startTask: false, + }) + + const child = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "child task", + parentTask: parent, + historyItem: { + id: "persisted-child", + ts: Date.now(), + task: "child task", + number: 1, + tokensIn: 0, + tokensOut: 0, + cacheWrites: 0, + cacheReads: 0, + totalCost: 0, + parentTaskId: parent.taskId, + depth: 7, + }, + startTask: false, + }) + + expect(child.depth).toBe(7) + expect(child.depthAuthoritative).toBe(true) + }) + + it("marks a legacy child resumed without its live parent as non-authoritative", () => { + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + historyItem: { + id: "legacy-child", + ts: Date.now(), + task: "legacy child", + number: 1, + tokensIn: 0, + tokensOut: 0, + cacheWrites: 0, + cacheReads: 0, + totalCost: 0, + parentTaskId: "missing-parent", + }, + startTask: false, + }) + + expect(task.depth).toBe(0) + expect(task.depthAuthoritative).toBe(false) + }) + }) }) describe("task-local configuration isolation", () => { diff --git a/src/core/task/__tests__/taskDepth.spec.ts b/src/core/task/__tests__/taskDepth.spec.ts new file mode 100644 index 0000000000..4802efff9f --- /dev/null +++ b/src/core/task/__tests__/taskDepth.spec.ts @@ -0,0 +1,85 @@ +// npx vitest core/task/__tests__/taskDepth.spec.ts + +import { computeTaskDepth, MAX_DEPTH_WALK } from "../taskDepth" + +describe("computeTaskDepth", () => { + it("returns a valid persisted depth as-is", () => { + expect(computeTaskDepth("a", 3, () => undefined)).toBe(3) + }) + + it("rejects non-integer / negative persisted depths and falls through to the walk", () => { + // ownDepth invalid -> walk; no parent -> root at depth 0 + expect(computeTaskDepth("a", -1, (id) => ({ parentTaskId: undefined }))).toBe(0) + expect(computeTaskDepth("a", 1.5, (id) => ({ parentTaskId: undefined }))).toBe(0) + }) + + it("derives depth from a live root with no persisted value", () => { + // c -> b -> a(root). No depths persisted anywhere. + const lookup = (id: string) => { + switch (id) { + case "c": + return { parentTaskId: "b" } + case "b": + return { parentTaskId: "a" } + case "a": + return { parentTaskId: undefined } + } + return undefined + } + expect(computeTaskDepth("c", undefined, lookup)).toBe(2) + }) + + it("derives depth from the nearest ancestor with a persisted value", () => { + // c -> b(depth 5) -> a. Only b has a persisted depth. + const lookup = (id: string) => { + switch (id) { + case "c": + return { parentTaskId: "b" } + case "b": + return { parentTaskId: "a", depth: 5 } + case "a": + return { parentTaskId: undefined, depth: 4 } + } + return undefined + } + expect(computeTaskDepth("c", undefined, lookup)).toBe(6) + }) + + it("returns undefined for a cycle in the parent chain", () => { + const lookup = (id: string) => { + switch (id) { + case "a": + return { parentTaskId: "b" } + case "b": + return { parentTaskId: "c" } + case "c": + return { parentTaskId: "a" } // cycle + } + return undefined + } + expect(computeTaskDepth("a", undefined, lookup)).toBeUndefined() + }) + + it("returns undefined when the chain dangles (parent not loadable)", () => { + const lookup = (id: string) => { + if (id === "c") return { parentTaskId: "missing" } + return undefined // "missing" cannot be loaded + } + expect(computeTaskDepth("c", undefined, lookup)).toBeUndefined() + }) + + it("returns undefined for a chain longer than MAX_DEPTH_WALK hops", () => { + // Build a linear chain of length > MAX_DEPTH_WALK with no persisted depths. + const nodes: Record = {} + for (let i = 0; i < MAX_DEPTH_WALK + 5; i++) { + nodes[`t${i}`] = { parentTaskId: i === 0 ? undefined : `t${i - 1}` } + } + const lookup = (id: string) => nodes[id] + expect(computeTaskDepth(`t${MAX_DEPTH_WALK + 4}`, undefined, lookup)).toBeUndefined() + }) + + it("handles a self-referencing parent", () => { + const lookup = (id: string) => ({ parentTaskId: id }) + expect(computeTaskDepth("a", undefined, lookup)).toBeUndefined() + }) +}) diff --git a/src/core/task/taskDepth.ts b/src/core/task/taskDepth.ts new file mode 100644 index 0000000000..265324ba0c --- /dev/null +++ b/src/core/task/taskDepth.ts @@ -0,0 +1,69 @@ +/** + * Pure helpers for task nesting depth. + * + * Depth is the number of delegation hops from the root task (root = 0). + * It is derived from the `parentTaskId` chain so that legacy tasks without a + * persisted `depth` field can be backfilled on load, and so that a corrupted + * or circular parent chain cannot produce an invalid depth. + */ + +/** Maximum ancestor hops to follow before giving up (guards against cycles). */ +export const MAX_DEPTH_WALK = 32 + +export type DepthLookup = (taskId: string) => { parentTaskId?: string; depth?: number } | undefined + +function isValidDepth(value: unknown): value is number { + return typeof value === "number" && Number.isInteger(value) && value >= 0 +} + +/** + * Compute the nesting depth for a task. + * + * - A persisted `depth` on the task itself is authoritative. + * - Otherwise, walk up the `parentTaskId` chain: + * - if an ancestor has a valid persisted depth, the task sits that many hops below it; + * - if we reach a root (no parent) without a persisted depth, the task's depth equals + * the number of hops taken to get there (a root is at depth 0); + * - on a cycle, a dangling reference, or an overly long chain, return `undefined` + * so callers fall back to treating the task as a root without persisting a bogus value. + */ +export function computeTaskDepth( + taskId: string, + ownDepth: number | undefined, + lookup: DepthLookup, +): number | undefined { + if (isValidDepth(ownDepth)) { + return ownDepth + } + + const seen = new Set([taskId]) + let currentId: string | undefined = taskId + let hopsFromTask = 0 + + while (hopsFromTask < MAX_DEPTH_WALK) { + const node: { parentTaskId?: string; depth?: number } | undefined = currentId ? lookup(currentId) : undefined + if (!node) { + // Parent chain references a task we cannot load — stop. + return undefined + } + if (isValidDepth(node.depth)) { + // Nearest ancestor with an authoritative depth: the original task sits + // `hopsFromTask` levels below it. + return node.depth + hopsFromTask + } + const parentId: string | undefined = node.parentTaskId + if (parentId === undefined) { + // Reached a root without a persisted depth (root = 0). + return hopsFromTask + } + if (seen.has(parentId)) { + // Cycle detected — refuse to persist a derived depth. + return undefined + } + seen.add(parentId) + currentId = parentId + hopsFromTask += 1 + } + + return undefined +} diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 20ff599a35..9dd872139c 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -103,6 +103,7 @@ import { ContextProxy } from "../config/ContextProxy" import { ProviderSettingsManager } from "../config/ProviderSettingsManager" import { CustomModesManager } from "../config/CustomModesManager" import { Task } from "../task/Task" +import { computeTaskDepth } from "../task/taskDepth" import { webviewMessageHandler } from "./webviewMessageHandler" import type { ClineMessage, TodoItem } from "@roo-code/types" @@ -1258,6 +1259,12 @@ export class ClineProvider diffFuzzyThreshold, }) + // Backfill the nesting depth for legacy tasks that lack a persisted value and were + // resumed without their live parent, so their first save persists a correct depth. + if (!task.depthAuthoritative) { + await this.backfillTaskDepth(task) + } + if (isRehydratingCurrentTask) { // Replace the current task in-place to avoid UI flicker const oldTask = this.taskRegistry.current @@ -1353,6 +1360,41 @@ export class ClineProvider return task } + /** + * Backfill the nesting depth for a resumed legacy task that lacks a persisted value. + * + * The Task constructor cannot derive depth when the live parent is not available (e.g. + * reopening from history), so it marks such tasks non-authoritative. Here we walk the + * `parentTaskId` chain through the in-memory store / global state and persist the + * computed depth so subsequent saves carry a correct value. A cycle or dangling parent + * reference yields no depth, leaving the task as-is rather than persisting a bogus one. + */ + private async backfillTaskDepth(task: Task): Promise { + if (task.depthAuthoritative) { + return + } + + const lookup = (id: string) => + this.taskHistoryStore.get(id) ?? (this.getGlobalState("taskHistory") ?? []).find((item) => item.id === id) + const depth = computeTaskDepth(task.taskId, undefined, lookup) + if (depth === undefined) { + return + } + + // Write back the complete existing record with `depth` added so no other fields are lost. + const existing = lookup(task.taskId) + if (!existing) { + return + } + try { + await this.updateTaskHistory({ ...existing, depth }, { broadcast: false }) + } catch (error) { + this.log( + `[backfillTaskDepth] Failed to persist backfilled depth for ${task.taskId}: ${error instanceof Error ? error.message : String(error)}`, + ) + } + } + public async postMessageToWebview(message: ExtensionMessage) { if (this._disposed) { return