-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: persist parent-child task relationships across extension reloads #6625
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -129,6 +129,10 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike { | |||||||||||||||||||||||||||||
| readonly taskNumber: number | ||||||||||||||||||||||||||||||
| readonly workspacePath: string | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // Store task IDs for persistence | ||||||||||||||||||||||||||||||
| readonly rootTaskId: string | undefined = undefined | ||||||||||||||||||||||||||||||
| readonly parentTaskId: string | undefined = undefined | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||
| * The mode associated with this task. Persisted across sessions | ||||||||||||||||||||||||||||||
| * to maintain user context when reopening tasks from history. | ||||||||||||||||||||||||||||||
|
|
@@ -307,6 +311,10 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike { | |||||||||||||||||||||||||||||
| this.parentTask = parentTask | ||||||||||||||||||||||||||||||
| this.taskNumber = taskNumber | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // Store task IDs for persistence | ||||||||||||||||||||||||||||||
| this.rootTaskId = rootTask?.taskId | ||||||||||||||||||||||||||||||
| this.parentTaskId = parentTask?.taskId | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // Store the task's mode when it's created. | ||||||||||||||||||||||||||||||
| // For history items, use the stored mode; for new tasks, we'll set it | ||||||||||||||||||||||||||||||
| // after getting state. | ||||||||||||||||||||||||||||||
|
|
@@ -582,6 +590,9 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike { | |||||||||||||||||||||||||||||
| globalStoragePath: this.globalStoragePath, | ||||||||||||||||||||||||||||||
| workspace: this.cwd, | ||||||||||||||||||||||||||||||
| mode: this._taskMode || defaultModeSlug, // Use the task's own mode, not the current provider mode | ||||||||||||||||||||||||||||||
| parentTaskId: this.parentTaskId, | ||||||||||||||||||||||||||||||
| rootTaskId: this.rootTaskId, | ||||||||||||||||||||||||||||||
| taskHierarchy: this.getTaskHierarchy(), | ||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| this.emit(RooCodeEventName.TaskTokenUsageUpdated, this.taskId, tokenUsage) | ||||||||||||||||||||||||||||||
|
|
@@ -2152,4 +2163,17 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike { | |||||||||||||||||||||||||||||
| public get cwd() { | ||||||||||||||||||||||||||||||
| return this.workspacePath | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // Get task hierarchy for persistence | ||||||||||||||||||||||||||||||
| public getTaskHierarchy(): string[] { | ||||||||||||||||||||||||||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This method could cause an infinite loop if task data becomes corrupted and creates circular references. Consider:
Suggested change
|
||||||||||||||||||||||||||||||
| const hierarchy: string[] = [] | ||||||||||||||||||||||||||||||
| let currentTask: Task | undefined = this.parentTask | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| while (currentTask) { | ||||||||||||||||||||||||||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding a maximum depth limit to prevent performance issues with excessively deep task hierarchies. You could add a MAX_HIERARCHY_DEPTH constant and check against it in the while loop. |
||||||||||||||||||||||||||||||
| hierarchy.unshift(currentTask.taskId) | ||||||||||||||||||||||||||||||
| currentTask = currentTask.parentTask | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| return hierarchy | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -740,6 +740,18 @@ export class ClineProvider | |||||||||||||||||||||||||
| experiments, | ||||||||||||||||||||||||||
| } = await this.getState() | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Restore parent and root tasks if their IDs are stored in the history item | ||||||||||||||||||||||||||
| let rootTask: Task | undefined = historyItem.rootTask | ||||||||||||||||||||||||||
| let parentTask: Task | undefined = historyItem.parentTask | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // If we don't have the actual task objects but have their IDs, try to find them in the stack | ||||||||||||||||||||||||||
| if (!rootTask && historyItem.rootTaskId) { | ||||||||||||||||||||||||||
| rootTask = this.clineStack.find((task) => task.taskId === historyItem.rootTaskId) | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| if (!parentTask && historyItem.parentTaskId) { | ||||||||||||||||||||||||||
| parentTask = this.clineStack.find((task) => task.taskId === historyItem.parentTaskId) | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const task = new Task({ | ||||||||||||||||||||||||||
| provider: this, | ||||||||||||||||||||||||||
| apiConfiguration, | ||||||||||||||||||||||||||
|
|
@@ -749,8 +761,8 @@ export class ClineProvider | |||||||||||||||||||||||||
| consecutiveMistakeLimit: apiConfiguration.consecutiveMistakeLimit, | ||||||||||||||||||||||||||
| historyItem, | ||||||||||||||||||||||||||
| experiments, | ||||||||||||||||||||||||||
| rootTask: historyItem.rootTask, | ||||||||||||||||||||||||||
| parentTask: historyItem.parentTask, | ||||||||||||||||||||||||||
| rootTask, | ||||||||||||||||||||||||||
| parentTask, | ||||||||||||||||||||||||||
| taskNumber: historyItem.number, | ||||||||||||||||||||||||||
| onCreated: (instance) => this.emit(RooCodeEventName.TaskCreated, instance), | ||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||
|
|
@@ -1344,6 +1356,23 @@ export class ClineProvider | |||||||||||||||||||||||||
| if (id !== this.getCurrentCline()?.taskId) { | ||||||||||||||||||||||||||
| // Non-current task. | ||||||||||||||||||||||||||
| const { historyItem } = await this.getTaskWithId(id) | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Check if this task has parent/child relationships that need to be restored | ||||||||||||||||||||||||||
| if (historyItem.taskHierarchy && historyItem.taskHierarchy.length > 0) { | ||||||||||||||||||||||||||
| // Restore the entire task hierarchy from root to this task | ||||||||||||||||||||||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding debug logging here to help troubleshoot hierarchy restoration issues in production:
Suggested change
|
||||||||||||||||||||||||||
| const taskHistory = this.getGlobalState("taskHistory") ?? [] | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // First, restore all parent tasks in the hierarchy | ||||||||||||||||||||||||||
| for (const taskId of historyItem.taskHierarchy) { | ||||||||||||||||||||||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing error handling here. If a parent task fails to restore, the entire hierarchy restoration could fail. Consider wrapping this in a try-catch:
Suggested change
|
||||||||||||||||||||||||||
| const parentHistoryItem = taskHistory.find((item: HistoryItem) => item.id === taskId) | ||||||||||||||||||||||||||
| if (parentHistoryItem && !this.clineStack.find((task) => task.taskId === taskId)) { | ||||||||||||||||||||||||||
| // This parent task is not in the stack, so restore it | ||||||||||||||||||||||||||
| await this.initClineWithHistoryItem(parentHistoryItem) | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Now restore the requested task | ||||||||||||||||||||||||||
| await this.initClineWithHistoryItem(historyItem) // Clears existing task. | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be helpful to add JSDoc comments explaining these new fields: