-
Notifications
You must be signed in to change notification settings - Fork 2.5k
fix: persist and restore active task on VS Code restart #7660
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 | ||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -353,6 +353,9 @@ export class ClineProvider | |||||||||||||||||||||||||||||||||||||||||||
| this.clineStack.push(task) | ||||||||||||||||||||||||||||||||||||||||||||
| task.emit(RooCodeEventName.TaskFocused) | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| // Persist the current active task ID | ||||||||||||||||||||||||||||||||||||||||||||
| await this.updateGlobalState("currentActiveTaskId", task.taskId) | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| // Perform special setup provider specific tasks. | ||||||||||||||||||||||||||||||||||||||||||||
| await this.performPreparationTasks(task) | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -417,6 +420,15 @@ export class ClineProvider | |||||||||||||||||||||||||||||||||||||||||||
| // garbage collected. | ||||||||||||||||||||||||||||||||||||||||||||
| task = undefined | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| // Clear the current active task ID if no tasks remain | ||||||||||||||||||||||||||||||||||||||||||||
| if (this.clineStack.length === 0) { | ||||||||||||||||||||||||||||||||||||||||||||
| await this.updateGlobalState("currentActiveTaskId", undefined) | ||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||
| // Update to the new top task | ||||||||||||||||||||||||||||||||||||||||||||
| const newCurrentTask = this.clineStack[this.clineStack.length - 1] | ||||||||||||||||||||||||||||||||||||||||||||
| await this.updateGlobalState("currentActiveTaskId", newCurrentTask.taskId) | ||||||||||||||||||||||||||||||||||||||||||||
|
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. Is this intentional? There's a potential race condition here between checking the stack length and updating the task ID. If another operation modifies the stack between lines 425 and 430, we might update to the wrong task ID. Consider using a lock or making this operation atomic. |
||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| getTaskStackSize(): number { | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -725,6 +737,22 @@ export class ClineProvider | |||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| // If the extension is starting a new session, clear previous task state. | ||||||||||||||||||||||||||||||||||||||||||||
| await this.removeClineFromStack() | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| // Attempt to restore the last active task if one was persisted | ||||||||||||||||||||||||||||||||||||||||||||
| const lastActiveTaskId = this.getGlobalState("currentActiveTaskId") | ||||||||||||||||||||||||||||||||||||||||||||
|
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. The task restoration logic could benefit from explicit cleanup if restoration fails partway through. Consider wrapping this in a try-finally block to ensure any partially initialized resources are properly released on error:
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||
| if (lastActiveTaskId && typeof lastActiveTaskId === "string") { | ||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||
| const { historyItem } = await this.getTaskWithId(lastActiveTaskId) | ||||||||||||||||||||||||||||||||||||||||||||
| if (historyItem) { | ||||||||||||||||||||||||||||||||||||||||||||
| await this.createTaskWithHistoryItem(historyItem) | ||||||||||||||||||||||||||||||||||||||||||||
| this.log(`Restored last active task: ${lastActiveTaskId}`) | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||
| // Task may have been deleted or corrupted, clear the saved ID | ||||||||||||||||||||||||||||||||||||||||||||
| this.log(`Failed to restore last active task ${lastActiveTaskId}: ${error}`) | ||||||||||||||||||||||||||||||||||||||||||||
|
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 more detailed logging here for debugging purposes. Including task details in the error log would help diagnose restoration issues:
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||
| await this.updateGlobalState("currentActiveTaskId", undefined) | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| public async createTaskWithHistoryItem(historyItem: HistoryItem & { rootTask?: Task; parentTask?: 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.
Could we add a more descriptive JSDoc comment here? Something like: