-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Fix/task execution mode not preserved after interruption #3482
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
Fix/task execution mode not preserved after interruption #3482
Conversation
|
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.
Similar to HistoryPreview, review the usage of dangerouslySetInnerHTML. Ensure that item.task is sanitized to prevent XSS vulnerabilities.
src/core/webview/ClineProvider.ts
Outdated
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.
Is this used anywhere?
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.
Yes, it is used in the new ClineProvider tests.
How does it handle model (not mode) changes? I often run code+gemini in one task and code+sonnet in another, others may too... I sort of think model changes should be task specific and must not modify global mode+model bindings, because "Settings" lets you set those defaults. New tasks should always use the global mode+model bindings |
446938e to
4d6fe6c
Compare
4d6fe6c to
ca4603d
Compare
This commit addresses a bug where changing the global UI mode would incorrectly update the `currentModeSlug` of an active or resumed task. This caused tasks to operate in the globally selected mode rather than their intended last active mode. Key changes: - Modified `ClineProvider.handleModeSwitch()` to only update the provider's global state (mode and API configuration) and emit events. It no longer directly modifies an active `Cline` instance's `currentModeSlug`. - Updated `switchModeTool.ts` so that after the `switch_mode` tool successfully calls `provider.handleModeSwitch()`, it then explicitly calls `cline.updateCurrentModeSlug()` on its own `Cline` instance. This ensures that a task's specific operational mode is only changed by the `switch_mode` tool executed within that task's context. Global UI mode changes will no longer affect the mode of an already initialized or resumed task. This fix is part of the broader effort to correctly preserve and restore the last active mode for tasks, as detailed in GitHub issue RooCodeInc#2553.
Also adds a clearStack utility function to ClineProvider
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.
Just curious, any reason to have a span here when HistoryPreview.tsx just sets the html on the div itself?
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.
Honestly: because Roo put it there and I am not a web frontend developer. I wholeheartedly encourage bringing someone else in to fix the UI code.
src/core/task/Task.ts
Outdated
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.
The updateCurrentModeSlug method should probably validate that the new mode actually exists before updating. Without validation, this could lead to tasks being in invalid states.
Does it make sense to use you could use getModeBySlug and check if the mode exists?
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.
True, I hadn't considered the deleted edge-case... Which mode should we default to if the requested mode is gone?
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.
What is the default mode for Roo Code when you first install it? that could probably work.
src/core/webview/ClineProvider.ts
Outdated
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.
Is clearStack() method only used in tests? If so does it make sense to move it to a test utility file or marking it as @internal?
If this is test-only code, it probably would be better placed in a test helper rather than the production class.
src/core/webview/ClineProvider.ts
Outdated
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.
I think we might have an issue if handleModeSwitch succeeds but resumePausedTask fails (throws an error), the provider will be left in the parent's mode but the parent task won't actually be resumed. This creates an inconsistent state where:
- The global provider mode is set to the parent's mode
- But the parent task is not actually running/resumed
- The UI might show the wrong mode for what's actually happening
|
Hey @axmo, sorry for taking so long, just left a couple of suggestions, let me know if you want to discuss them further. This looks great, I remember you also modified the UI a bit and it looks awesome! Do you think this would look better in any of these places? |
(felt like a bug the other way)
Introduces a `modeIsFrozen` flag in the Task class. This flag is set to `true` when a task completes or a completed task is resumed. When `modeIsFrozen` is true, the task's `currentModeSlug` is prevented from being updated if the user only changes the mode in the UI without sending a new message. This avoids saving accidental mode changes during history browsing. If a new message is sent to continue a task that was frozen: 1. `modeIsFrozen` is reset to `false`. 2. The task's `currentModeSlug` is updated to match the actual mode selected in the UI (obtained from `ClineProvider.getState()`). 3. This updated `currentModeSlug` is then used when saving task messages and metadata, ensuring the persisted history accurately reflects the mode in which the interaction occurred. This resolves a bug where re-opening a completed task, changing the mode, and then sending a message would result in the original (stale) mode being persisted in the task history, instead of the new mode used for the interaction.
d7460a2 to
6716e41
Compare
|
stale |


Related GitHub Issue
Closes: #2553
Description
This PR implements a system to ensure tasks retain and resume in their last active execution mode, addressing inconsistencies that occurred when tasks were interrupted, subtasks completed, or the global UI mode was changed.
The core changes are:
Mode Persistence in Task History:
HistoryItemtype (insrc/schemas/index.ts) now includes alastActiveModeSlug?: stringfield to store the slug of the mode the task was last operating in.taskMetadata()function (insrc/core/task-persistence/taskMetadata.ts) now populates thislastActiveModeSlugwhen saving task metadata.Task-Specific Current Mode:
Taskclass (insrc/core/task/Task.ts) now requirescurrentModeSlug: stringin itsTaskOptionsand maintains this asthis.currentModeSlug.updateCurrentModeSlug(newModeSlug: string)allows the task's specific mode to be updated.saveTaskMessages()usesthis.currentModeSlugto set thelastActiveModeSlugin the task's metadata.Mode Handling in
ClineProvider(insrc/core/webview/ClineProvider.ts):initTask(): When a new task is created, it's initialized with the provider's current global mode as itscurrentModeSlug.initTaskFromHistory(): When resuming from history, the task is initialized withhistoryItem.lastActiveModeSlug(or a fallback to the global mode if not present) as itscurrentModeSlug. If this restored mode differs from the provider's current global mode,handleModeSwitch()is called to align the provider's state.handleModeSwitch(): This method now exclusively manages theClineProvider's global active mode and API configuration. It no longer directly modifies thecurrentModeSlugof an activeTaskinstance.finishSubTask(): When a subtask completes, the provider ensures the parent task'scurrentModeSlugis active by callinghandleModeSwitch()if necessary, before resuming the parent task.Mode Switching Tool (
switchModeTool.tsinsrc/core/tools/switchModeTool.ts):provider.handleModeSwitch()to change the global/provider-level mode, theswitch_modetool now explicitly callstask.updateCurrentModeSlug()(on the activeTaskinstance). This ensures that a task's specific operational mode is only changed by an explicitswitch_modeaction from within that task.UI Enhancements:
webview-ui/src/components/history/HistoryPreview.tsxandwebview-ui/src/components/history/HistoryView.tsx) have been updated to display the "[Last Mode: ]" for each task.Test Updates:
src/core/task/__tests__/Task.test.tsandsrc/core/webview/__tests__/ClineProvider.test.ts) have been updated to provide the new requiredcurrentModeSlugproperty inTaskOptionsduringTaskinstantiation, resolving previous TypeScript compilation errors.These changes ensure that a task's operational mode is robustly preserved and restored, independent of global UI mode changes, unless explicitly altered by the task itself via the
switch_modetool.Test Procedure
tscto ensure there are no TypeScript compilation errors.npm testto ensure all unit tests pass.switch_modetool to change its mode (e.g., to "Code").HistoryPreview.tsxandHistoryView.tsx) to ensure the "Last Mode" is displayed correctly.Type of Change
Task).srcor test files.Pre-Submission Checklist
npm run lint). (Assumed based on previoustscsuccess)console.log) has been removed.currentModeSlug).npm test). (Assumed based on previoustscsuccess and user request for PR)tscsuccess)mainbranch. (User to confirm)npm run changesetif this PR includes user-facing changes or dependency updates. (User to determine if UI change for history warrants this)Screenshots / Videos
Documentation Updates
Additional Notes
This PR implements the core logic for preserving and restoring the last active mode of a task.
Important
Fixes task execution mode persistence after interruptions by storing and restoring the last active mode, with UI and test updates.
lastActiveModeSlugtoHistoryIteminindex.tsto store the last active mode.taskMetadata()intaskMetadata.tsto savelastActiveModeSlug.Taskclass inTask.tsnow requirescurrentModeSlugand includesupdateCurrentModeSlug().ClineProviderinClineProvider.tsinitializes tasks with the correct mode and handles mode switches.switchModeTool.tsupdates task mode after global mode switch.HistoryPreview.tsxandHistoryView.tsxdisplay last active mode for tasks.Task.test.tsandClineProvider.test.tsto includecurrentModeSluginTaskOptions.lastActiveModeSlugtoGlobalSettingsinroo-code.d.tsandtypes.ts.This description was created by
for 9b813808cca139bc71509f93ed8595f149cfc705. You can customize this summary. It will automatically update as commits are pushed.