Skip to content

Commit 073e6a3

Browse files
hannesrudolphhannesrudolph
committed
fix: restore API configuration when restoring task from history with mode
- When a task is restored from history with a saved mode, now also restore the API configuration associated with that mode - This fixes the issue where spawned tasks would use the parent task's API configuration after reload instead of their own - Added test to verify API configuration is restored correctly
1 parent efc7b09 commit 073e6a3

File tree

10 files changed

+512
-2
lines changed

10 files changed

+512
-2
lines changed

‎.roo/temp/pr-6177/comments.json‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
[
2+
{
3+
"author": { "login": "hannesrudolph" },
4+
"authorAssociation": "MEMBER",
5+
"body": "@roomote-agent use pr reviewer",
6+
"createdAt": "2025-07-25T01:32:22Z",
7+
"id": "IC_kwDONIq5lM65taaz",
8+
"includesCreatedEdit": false,
9+
"isMinimized": false,
10+
"minimizedReason": "",
11+
"reactionGroups": [],
12+
"url": "https://github.com/RooCodeInc/Roo-Code/pull/6177#issuecomment-3115689651",
13+
"viewerDidAuthor": true
14+
},
15+
{
16+
"author": { "login": "roomote" },
17+
"authorAssociation": "NONE",
18+
"body": "I see your request to review this PR. Let me switch to PR reviewer mode to provide a comprehensive review of the sticky mode implementation.",
19+
"createdAt": "2025-07-25T01:34:02Z",
20+
"id": "IC_kwDONIq5lM65tfHL",
21+
"includesCreatedEdit": false,
22+
"isMinimized": false,
23+
"minimizedReason": "",
24+
"reactionGroups": [],
25+
"url": "https://github.com/RooCodeInc/Roo-Code/pull/6177#issuecomment-3115708875",
26+
"viewerDidAuthor": false
27+
},
28+
{
29+
"author": { "login": "hannesrudolph" },
30+
"authorAssociation": "MEMBER",
31+
"body": "@roomote-agent run pr reviewer",
32+
"createdAt": "2025-07-25T01:59:51Z",
33+
"id": "IC_kwDONIq5lM65ujXI",
34+
"includesCreatedEdit": false,
35+
"isMinimized": false,
36+
"minimizedReason": "",
37+
"reactionGroups": [],
38+
"url": "https://github.com/RooCodeInc/Roo-Code/pull/6177#issuecomment-3115988424",
39+
"viewerDidAuthor": true
40+
},
41+
{
42+
"author": { "login": "roomote" },
43+
"authorAssociation": "NONE",
44+
"body": "I see your request to run PR reviewer mode. Let me switch to that mode and provide a comprehensive review of this sticky mode implementation.",
45+
"createdAt": "2025-07-25T02:01:23Z",
46+
"id": "IC_kwDONIq5lM65unVS",
47+
"includesCreatedEdit": false,
48+
"isMinimized": false,
49+
"minimizedReason": "",
50+
"reactionGroups": [],
51+
"url": "https://github.com/RooCodeInc/Roo-Code/pull/6177#issuecomment-3116004690",
52+
"viewerDidAuthor": false
53+
}
54+
]

‎.roo/temp/pr-6177/final-review.md‎

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
# PR Review Summary for #6177: Implement Sticky Task Mode
2+
3+
## Executive Summary
4+
5+
This PR implements a "sticky mode" feature that persists the last used mode when reopening a task from history. While the implementation is functional and follows existing patterns, there are critical issues with test coverage and some minor architectural concerns that should be addressed.
6+
7+
## Critical Issues (must fix)
8+
9+
### 1. **Breaking Change in new_task Tool**
10+
11+
The current implementation has a critical issue with the `new_task` tool that will break with sticky mode:
12+
13+
**Problem:** In [`newTaskTool.ts`](src/core/tools/newTaskTool.ts:84), the mode is switched BEFORE creating the new task:
14+
15+
```typescript
16+
// Line 84: Switch mode first, then create new task instance.
17+
await provider.handleModeSwitch(mode)
18+
```
19+
20+
With sticky mode, this means:
21+
22+
1. Parent task switches to the new mode
23+
2. Parent task's mode is saved as the new mode (not its original mode)
24+
3. When the subtask completes and parent resumes, it will have the wrong mode
25+
26+
**Required Fix:** The mode switch must happen AFTER the new task is created:
27+
28+
```typescript
29+
// Create new task first (preserving parent's current mode)
30+
const newCline = await provider.initClineWithTask(unescapedMessage, undefined, cline)
31+
32+
// Then switch the new task to the desired mode
33+
await provider.handleModeSwitch(mode)
34+
```
35+
36+
This ensures the parent task's mode is preserved correctly in its history.
37+
38+
### 2. **Missing Test Coverage for Core Functionality**
39+
40+
The PR lacks specific tests for the new sticky mode feature:
41+
42+
- No tests verify that the mode is correctly saved to `taskMetadata` when switching modes
43+
- No tests verify that the mode is restored when reopening a task from history
44+
- The existing test files (`Task.spec.ts` and `ClineProvider.spec.ts`) were not updated to cover the new functionality
45+
46+
**Evidence:**
47+
48+
- Searched for `handleModeSwitch`, `initClineWithHistoryItem`, and `taskMetadata` in test files
49+
- Found no tests specifically validating the persistence and restoration of mode in task metadata
50+
51+
**Recommendation:** Add comprehensive test coverage:
52+
53+
```typescript
54+
// In ClineProvider.spec.ts
55+
it("should save mode to task metadata when switching modes", async () => {
56+
// Test that handleModeSwitch updates task history with new mode
57+
})
58+
59+
it("should restore mode from history item when reopening task", async () => {
60+
// Test that initClineWithHistoryItem restores the saved mode
61+
})
62+
63+
// In Task.spec.ts
64+
it("should include current mode in task metadata", async () => {
65+
// Test that saveClineMessages includes mode in metadata
66+
})
67+
```
68+
69+
## Pattern Inconsistencies
70+
71+
### 1. **Inconsistent Mode Persistence Approach**
72+
73+
The implementation adds mode persistence at the task level, but the codebase already has a pattern for persisting mode-specific API configurations (`modeApiConfigs`). This creates two different approaches for mode-related persistence.
74+
75+
**Current patterns:**
76+
77+
- Mode-specific API configs: `providerSettingsManager.setModeConfig()`
78+
- Task-specific mode: Added to `HistoryItem` and `taskMetadata`
79+
80+
**Recommendation:** Document why task-level mode persistence is needed in addition to the existing mode configuration system.
81+
82+
## Architecture Concerns
83+
84+
### 1. **Tight Coupling Between Task and Mode**
85+
86+
The implementation creates a direct dependency between tasks and modes by adding the `mode` field to `HistoryItem`. This could make it harder to refactor the mode system in the future.
87+
88+
**Consider:** Using a more flexible approach like storing mode in a separate metadata object that could be extended with other task preferences in the future.
89+
90+
## Minor Suggestions
91+
92+
### 1. **Type Safety Enhancement**
93+
94+
The `mode` field is typed as `string` in both `HistoryItem` and `taskMetadata`. Consider using the `Mode` type for better type safety:
95+
96+
```typescript
97+
// In packages/types/src/history.ts
98+
mode: z.enum(['code', 'architect', 'ask', 'debug', ...]).optional()
99+
```
100+
101+
### 2. **Documentation**
102+
103+
Add JSDoc comments to explain the purpose of the mode field:
104+
105+
```typescript
106+
/**
107+
* The mode that was active when this task was last saved.
108+
* Used to restore the user's preferred mode when reopening the task.
109+
*/
110+
mode?: string
111+
```
112+
113+
## Test Coverage Issues
114+
115+
The PR claims "All existing tests pass" but provides no new tests for the feature. This is concerning for a feature that modifies core task persistence behavior.
116+
117+
**Required tests:**
118+
119+
1. Mode is saved when task history is updated
120+
2. Mode is restored when task is reopened
121+
3. Mode persistence works correctly with subtasks
122+
4. Null/undefined mode is handled gracefully
123+
124+
## Summary
125+
126+
This PR has two critical issues that must be addressed:
127+
128+
1. **The new_task tool implementation is incompatible with sticky mode** - The mode switch happens before task creation, which will cause the parent task to save the wrong mode. This is a breaking change that will affect subtask functionality.
129+
130+
2. **Complete lack of test coverage** - The feature modifies important task persistence behavior without any tests to ensure it works correctly or to prevent regressions.
131+
132+
**Verdict:** Request changes - Fix the new_task tool implementation and add comprehensive test coverage before merging.
133+
134+
## Additional Implementation Notes
135+
136+
The fix for the new_task tool should:
137+
138+
1. Create the new task first (which will preserve the parent's current mode in its history)
139+
2. Then switch the newly created task to the desired mode
140+
3. This ensures parent tasks maintain their original mode when resuming after subtasks complete

‎.roo/temp/pr-6177/pr-metadata.json‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
{
2+
"additions": 22,
3+
"author": { "is_bot": true, "login": "app/roomote" },
4+
"baseRefName": "main",
5+
"body": "## Summary\n\nThis PR implements sticky mode functionality for tasks, ensuring that when a task is reopened from history, it automatically switches to the last mode that was active on that task.\n\n## Changes\n\n- Added `mode` field to `HistoryItem` schema to persist the selected mode\n- Updated `taskMetadata` function to save the current mode when creating history items\n- Modified `handleModeSwitch` to update task history with the new mode when switching modes on an active task\n- Updated `initClineWithHistoryItem` to restore the saved mode when reopening tasks from history\n\n## Behavior\n\n1. When a task is running and the user switches modes, the new mode is saved to the task's history\n2. When a task is reopened from history, it automatically switches to the last saved mode\n3. If a task has no saved mode (e.g., older tasks), it will use the current default mode\n\n## Testing\n\n- All existing tests pass ✅\n- Type checking passes ✅\n- Linting passes ✅\n\nThis feature improves the user experience by maintaining context when switching between tasks, eliminating the need to manually switch back to the desired mode after reopening a task.\n<!-- ELLIPSIS_HIDDEN -->\n\n----\n\n> [!IMPORTANT]\n> Introduces sticky mode for tasks, saving and restoring the last used mode across sessions by updating task history and metadata handling.\n> \n> - **Behavior**:\n> - Tasks now save the current mode to history when switching modes (`handleModeSwitch` in `ClineProvider`).\n> - When reopening tasks from history, the last saved mode is restored (`initClineWithHistoryItem` in `ClineProvider`).\n> - If no mode is saved, the default mode is used.\n> - **Schema**:\n> - Added `mode` field to `HistoryItem` schema in `history.ts`.\n> - **Functions**:\n> - Updated `taskMetadata` in `taskMetadata.ts` to include mode when creating history items.\n> - Modified `Task` class to save mode in task history when saving messages.\n> - **Testing**:\n> - All existing tests, type checking, and linting pass.\n> \n> <sup>This description was created by </sup>[<img alt=\"Ellipsis\" src=\"https://img.shields.io/badge/Ellipsis-blue?color=175173\">](https://www.ellipsis.dev?ref=RooCodeInc%2FRoo-Code&utm_source=github&utm_medium=referral)<sup> for a4b0ad356d9156b4cfa6bf64b8f1db6cacae48b3. You can [customize](https://app.ellipsis.dev/RooCodeInc/settings/summaries) this summary. It will automatically update as commits are pushed.</sup>\n\n<!-- ELLIPSIS_HIDDEN -->",
6+
"changedFiles": 4,
7+
"deletions": 0,
8+
"files": [
9+
{ "path": "packages/types/src/history.ts", "additions": 1, "deletions": 0 },
10+
{ "path": "src/core/task-persistence/taskMetadata.ts", "additions": 3, "deletions": 0 },
11+
{ "path": "src/core/task/Task.ts", "additions": 5, "deletions": 0 },
12+
{ "path": "src/core/webview/ClineProvider.ts", "additions": 13, "deletions": 0 }
13+
],
14+
"headRefName": "feature/sticky-mode-for-tasks",
15+
"number": 6177,
16+
"state": "OPEN",
17+
"title": "feat: make task mode sticky across sessions",
18+
"url": "https://github.com/RooCodeInc/Roo-Code/pull/6177"
19+
}

‎.roo/temp/pr-6177/pr.diff‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
diff --git a/packages/types/src/history.ts b/packages/types/src/history.ts
2+
index 8c75024879c..ace134566e3 100644
3+
--- a/packages/types/src/history.ts
4+
+++ b/packages/types/src/history.ts
5+
@@ -16,6 +16,7 @@ export const historyItemSchema = z.object({
6+
totalCost: z.number(),
7+
size: z.number().optional(),
8+
workspace: z.string().optional(),
9+
+ mode: z.string().optional(),
10+
})
11+
12+
export type HistoryItem = z.infer<typeof historyItemSchema>
13+
diff --git a/src/core/task-persistence/taskMetadata.ts b/src/core/task-persistence/taskMetadata.ts
14+
index 1759a72f475..7b93b5c14a0 100644
15+
--- a/src/core/task-persistence/taskMetadata.ts
16+
+++ b/src/core/task-persistence/taskMetadata.ts
17+
@@ -18,6 +18,7 @@ export type TaskMetadataOptions = {
18+
taskNumber: number
19+
globalStoragePath: string
20+
workspace: string
21+
+ mode?: string
22+
}
23+
24+
export async function taskMetadata({
25+
@@ -26,6 +27,7 @@ export async function taskMetadata({
26+
taskNumber,
27+
globalStoragePath,
28+
workspace,
29+
+ mode,
30+
}: TaskMetadataOptions) {
31+
const taskDir = await getTaskDirectoryPath(globalStoragePath, taskId)
32+
33+
@@ -92,6 +94,7 @@ export async function taskMetadata({
34+
totalCost: tokenUsage.totalCost,
35+
size: taskDirSize,
36+
workspace,
37+
+ mode,
38+
}
39+
40+
return { historyItem, tokenUsage }
41+
diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts
42+
index 95d12f66aa1..ce42970a3dd 100644
43+
--- a/src/core/task/Task.ts
44+
+++ b/src/core/task/Task.ts
45+
@@ -405,12 +405,17 @@ export class Task extends EventEmitter<ClineEvents> {
46+
globalStoragePath: this.globalStoragePath,
47+
})
48+
49+
+ const provider = this.providerRef.deref()
50+
+ const state = await provider?.getState()
51+
+ const currentMode = state?.mode
52+
+
53+
const { historyItem, tokenUsage } = await taskMetadata({
54+
messages: this.clineMessages,
55+
taskId: this.taskId,
56+
taskNumber: this.taskNumber,
57+
globalStoragePath: this.globalStoragePath,
58+
workspace: this.cwd,
59+
+ mode: currentMode,
60+
})
61+
62+
this.emit("taskTokenUsageUpdated", this.taskId, tokenUsage)
63+
diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts
64+
index 905e657b37e..cb32b05a684 100644
65+
--- a/src/core/webview/ClineProvider.ts
66+
+++ b/src/core/webview/ClineProvider.ts
67+
@@ -578,6 +578,11 @@ export class ClineProvider
68+
public async initClineWithHistoryItem(historyItem: HistoryItem & { rootTask?: Task; parentTask?: Task }) {
69+
await this.removeClineFromStack()
70+
71+
+ // If the history item has a saved mode, restore it
72+
+ if (historyItem.mode) {
73+
+ await this.updateGlobalState("mode", historyItem.mode)
74+
+ }
75+
+
76+
const {
77+
apiConfiguration,
78+
diffEnabled: enableDiff,
79+
@@ -807,6 +812,14 @@ export class ClineProvider
80+
if (cline) {
81+
TelemetryService.instance.captureModeSwitch(cline.taskId, newMode)
82+
cline.emit("taskModeSwitched", cline.taskId, newMode)
83+
+
84+
+ // Update the task history with the new mode
85+
+ const history = this.getGlobalState("taskHistory") ?? []
86+
+ const taskHistoryItem = history.find((item) => item.id === cline.taskId)
87+
+ if (taskHistoryItem) {
88+
+ taskHistoryItem.mode = newMode
89+
+ await this.updateTaskHistory(taskHistoryItem)
90+
+ }
91+
}
92+
93+
await this.updateGlobalState("mode", newMode)

‎.roo/temp/pr-6177/review-context.json‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
{
2+
"prNumber": "6177",
3+
"repository": "RooCodeInc/Roo-Code",
4+
"reviewStartTime": "2025-01-27T16:15:32.743Z",
5+
"calledByMode": null,
6+
"prMetadata": {
7+
"title": "feat: make task mode sticky across sessions",
8+
"author": "app/roomote",
9+
"state": "OPEN",
10+
"baseRefName": "main",
11+
"headRefName": "feature/sticky-mode-for-tasks",
12+
"additions": 22,
13+
"deletions": 0,
14+
"changedFiles": 4
15+
},
16+
"linkedIssue": {},
17+
"existingComments": [],
18+
"existingReviews": [],
19+
"filesChanged": [
20+
"packages/types/src/history.ts",
21+
"src/core/task-persistence/taskMetadata.ts",
22+
"src/core/task/Task.ts",
23+
"src/core/webview/ClineProvider.ts"
24+
],
25+
"delegatedTasks": [],
26+
"findings": {
27+
"critical": [],
28+
"patterns": [],
29+
"redundancy": [],
30+
"architecture": [],
31+
"tests": []
32+
},
33+
"reviewStatus": "analyzing"
34+
}

‎.roo/temp/pr-6177/reviews.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
[]

‎src/core/webview/ClineProvider.ts‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -578,9 +578,25 @@ export class ClineProvider
578578
public async initClineWithHistoryItem(historyItem: HistoryItem & { rootTask?: Task; parentTask?: Task }) {
579579
await this.removeClineFromStack()
580580

581-
// If the history item has a saved mode, restore it
581+
// If the history item has a saved mode, restore it and its associated API configuration
582582
if (historyItem.mode) {
583583
await this.updateGlobalState("mode", historyItem.mode)
584+
585+
// Load the saved API config for the restored mode if it exists
586+
const savedConfigId = await this.providerSettingsManager.getModeConfigId(historyItem.mode)
587+
const listApiConfig = await this.providerSettingsManager.listConfig()
588+
589+
// Update listApiConfigMeta first to ensure UI has latest data
590+
await this.updateGlobalState("listApiConfigMeta", listApiConfig)
591+
592+
// If this mode has a saved config, use it
593+
if (savedConfigId) {
594+
const profile = listApiConfig.find(({ id }) => id === savedConfigId)
595+
596+
if (profile?.name) {
597+
await this.activateProviderProfile({ name: profile.name })
598+
}
599+
}
584600
}
585601

586602
const {

0 commit comments

Comments
 (0)