Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions evals/packages/types/src/roo-code.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ export const historyItemSchema = z.object({
totalCost: z.number(),
size: z.number().optional(),
workspace: z.string().optional(),
lastActiveModeSlug: z.string().optional(),

})

export type HistoryItem = z.infer<typeof historyItemSchema>
Expand Down
1 change: 1 addition & 0 deletions packages/types/src/history.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export const historyItemSchema = z.object({
totalCost: z.number(),
size: z.number().optional(),
workspace: z.string().optional(),
lastActiveModeSlug: z.string().optional(),
})

export type HistoryItem = z.infer<typeof historyItemSchema>
3 changes: 3 additions & 0 deletions src/core/task-persistence/taskMetadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export type TaskMetadataOptions = {
taskNumber: number
globalStoragePath: string
workspace: string
lastActiveModeSlug?: string
}

export async function taskMetadata({
Expand All @@ -25,6 +26,7 @@ export async function taskMetadata({
taskNumber,
globalStoragePath,
workspace,
lastActiveModeSlug,
}: TaskMetadataOptions) {
const taskDir = await getTaskDirectoryPath(globalStoragePath, taskId)
const taskMessage = messages[0] // First message is always the task say.
Expand Down Expand Up @@ -57,6 +59,7 @@ export async function taskMetadata({
totalCost: tokenUsage.totalCost,
size: taskDirSize,
workspace,
lastActiveModeSlug,
}

return { historyItem, tokenUsage }
Expand Down
40 changes: 39 additions & 1 deletion src/core/task/Task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ export type TaskOptions = {
parentTask?: Task
taskNumber?: number
onCreated?: (cline: Task) => void
currentModeSlug: string
}

export class Task extends EventEmitter<ClineEvents> {
Expand All @@ -123,7 +124,8 @@ export class Task extends EventEmitter<ClineEvents> {
readonly parentTask: Task | undefined = undefined
readonly taskNumber: number
readonly workspacePath: string

currentModeSlug: string
modeIsFrozen: boolean = false
providerRef: WeakRef<ClineProvider>
private readonly globalStoragePath: string
abort: boolean = false
Expand Down Expand Up @@ -205,6 +207,7 @@ export class Task extends EventEmitter<ClineEvents> {
parentTask,
taskNumber = -1,
onCreated,
currentModeSlug,
}: TaskOptions) {
super()

Expand Down Expand Up @@ -239,6 +242,7 @@ export class Task extends EventEmitter<ClineEvents> {
this.globalStoragePath = provider.context.globalStorageUri.fsPath
this.diffViewProvider = new DiffViewProvider(this.cwd)
this.enableCheckpoints = enableCheckpoints
this.currentModeSlug = currentModeSlug

this.rootTask = rootTask
this.parentTask = parentTask
Expand Down Expand Up @@ -294,6 +298,17 @@ export class Task extends EventEmitter<ClineEvents> {
await this.saveApiConversationHistory()
}

public async updateCurrentModeSlug(newModeSlug: string) {
Copy link
Member

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?

Copy link
Contributor Author

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?

Copy link
Member

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.

if (this.modeIsFrozen) {
// If the mode is frozen, ignore any attempts to change it.
return // Crucial: Exit without updating the mode
}
// Only update if the new mode is actually different to avoid unnecessary assignments
if (this.currentModeSlug !== newModeSlug) {
this.currentModeSlug = newModeSlug
}
}

async overwriteApiConversationHistory(newHistory: ApiMessage[]) {
this.apiConversationHistory = newHistory
await this.saveApiConversationHistory()
Expand Down Expand Up @@ -367,6 +382,7 @@ export class Task extends EventEmitter<ClineEvents> {
taskNumber: this.taskNumber,
globalStoragePath: this.globalStoragePath,
workspace: this.cwd,
lastActiveModeSlug: this.currentModeSlug,
})

this.emit("taskTokenUsageUpdated", this.taskId, tokenUsage)
Expand Down Expand Up @@ -472,6 +488,14 @@ export class Task extends EventEmitter<ClineEvents> {
await this.addToClineMessages({ ts: askTs, type: "ask", ask: type, text })
}

// Set modeIsFrozen if the task just completed
if (
(type === "completion_result" || type === "resume_completed_task") &&
(partial === false || partial === undefined)
) {
this.modeIsFrozen = true
}

await pWaitFor(() => this.askResponse !== undefined || this.lastMessageTs !== askTs, { interval: 100 })

if (this.lastMessageTs !== askTs) {
Expand Down Expand Up @@ -1069,6 +1093,20 @@ export class Task extends EventEmitter<ClineEvents> {
throw new Error(`[RooCode#recursivelyMakeRooRequests] task ${this.taskId}.${this.instanceId} aborted`)
}

// Reset modeIsFrozen when a new message is sent
if (this.modeIsFrozen) {
this.modeIsFrozen = false
// Get the actual current mode from the provider and update the task's mode
const provider = this.providerRef.deref()
if (provider) {
const providerState = await provider.getState()
if (providerState.mode !== this.currentModeSlug) {
this.currentModeSlug = providerState.mode
}
}
await this.saveClineMessages() // Save messages now reflects the unfreezing and potential mode update
}

if (this.consecutiveMistakeCount >= this.consecutiveMistakeLimit) {
const { response, text, images } = await this.ask(
"mistake_limit_reached",
Expand Down
10 changes: 9 additions & 1 deletion src/core/task/__tests__/Task.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@ describe("Cline", () => {
const cline = new Task({
provider: mockProvider,
apiConfiguration: mockApiConfig,
currentModeSlug: "code",
fuzzyMatchThreshold: 0.95,
task: "test task",
startTask: false,
Expand All @@ -289,6 +290,7 @@ describe("Cline", () => {
const cline = new Task({
provider: mockProvider,
apiConfiguration: mockApiConfig,
currentModeSlug: "code",
enableDiff: true,
fuzzyMatchThreshold: 0.95,
task: "test task",
Expand All @@ -303,7 +305,7 @@ describe("Cline", () => {

it("should require either task or historyItem", () => {
expect(() => {
new Task({ provider: mockProvider, apiConfiguration: mockApiConfig })
new Task({ provider: mockProvider, apiConfiguration: mockApiConfig, currentModeSlug: "code" })
}).toThrow("Either historyItem or task/images must be provided")
})
})
Expand All @@ -315,6 +317,7 @@ describe("Cline", () => {
const [cline, task] = Task.create({
provider: mockProvider,
apiConfiguration: mockApiConfig,
currentModeSlug: "code", // Added for test
task: "test task",
})

Expand Down Expand Up @@ -423,6 +426,7 @@ describe("Cline", () => {
const [clineWithImages, taskWithImages] = Task.create({
provider: mockProvider,
apiConfiguration: configWithImages,
currentModeSlug: "code", // Added for test
task: "test task",
})

Expand All @@ -446,6 +450,7 @@ describe("Cline", () => {
const [clineWithoutImages, taskWithoutImages] = Task.create({
provider: mockProvider,
apiConfiguration: configWithoutImages,
currentModeSlug: "code", // Added for test
task: "test task",
})

Expand Down Expand Up @@ -537,6 +542,7 @@ describe("Cline", () => {
const [cline, task] = Task.create({
provider: mockProvider,
apiConfiguration: mockApiConfig,
currentModeSlug: "code", // Added for test
task: "test task",
})

Expand Down Expand Up @@ -662,6 +668,7 @@ describe("Cline", () => {
const [cline, task] = Task.create({
provider: mockProvider,
apiConfiguration: mockApiConfig,
currentModeSlug: "code", // Added for test
task: "test task",
})

Expand Down Expand Up @@ -787,6 +794,7 @@ describe("Cline", () => {
const [cline, task] = Task.create({
provider: mockProvider,
apiConfiguration: mockApiConfig,
currentModeSlug: "code", // Added for test
task: "test task",
})

Expand Down
12 changes: 11 additions & 1 deletion src/core/tools/switchModeTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,17 @@ export async function switchModeTool(
}

// Switch the mode using shared handler
await cline.providerRef.deref()?.handleModeSwitch(mode_slug)
const provider = cline.providerRef.deref()
if (provider) {
await provider.handleModeSwitch(mode_slug)
// After provider's global mode is switched, update this Cline instance's currentModeSlug
await cline.updateCurrentModeSlug(mode_slug)
} else {
// Should not happen, but handle gracefully
cline.recordToolError("switch_mode")
pushToolResult(formatResponse.toolError("Failed to get provider reference for mode switch."))
return
}

pushToolResult(
`Successfully switched from ${getModeBySlug(currentMode)?.name ?? currentMode} mode to ${
Expand Down
41 changes: 38 additions & 3 deletions src/core/webview/ClineProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,15 @@ export class ClineProvider
}
}

public async clearStack() {
Copy link
Member

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.

this.log("Clearing entire cline stack")
while (this.clineStack.length > 0) {
// removeClineFromStack already logs the removal of each task
await this.removeClineFromStack()
}
this.log("Cline stack cleared")
}
Comment on lines +195 to +202
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this used anywhere?

Copy link
Contributor Author

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.


// returns the current cline object in the stack (the top one)
// if the stack is empty, returns undefined
getCurrentCline(): Task | undefined {
Expand All @@ -217,8 +226,17 @@ export class ClineProvider
console.log(`[subtasks] finishing subtask ${lastMessage}`)
// remove the last cline instance from the stack (this is the finished sub task)
await this.removeClineFromStack()
// resume the last cline instance in the stack (if it exists - this is the 'parent' calling task)
await this.getCurrentCline()?.resumePausedTask(lastMessage)

const parentCline = this.getCurrentCline()
if (parentCline) {
const parentLastActiveMode = parentCline.currentModeSlug // Changed from initialModeSlug
const currentActiveMode = (await this.getState()).mode

if (parentLastActiveMode && parentLastActiveMode !== currentActiveMode) {
Copy link
Member

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

await this.handleModeSwitch(parentLastActiveMode)
}
await parentCline.resumePausedTask(lastMessage)
}
}

/*
Expand Down Expand Up @@ -517,6 +535,7 @@ export class ClineProvider
diffEnabled: enableDiff,
enableCheckpoints,
fuzzyMatchThreshold,
mode,
experiments,
} = await this.getState()

Expand All @@ -537,6 +556,7 @@ export class ClineProvider
parentTask,
taskNumber: this.clineStack.length + 1,
onCreated: (cline) => this.emit("clineCreated", cline),
currentModeSlug: mode,
...options,
})

Expand All @@ -552,6 +572,17 @@ export class ClineProvider
public async initClineWithHistoryItem(historyItem: HistoryItem & { rootTask?: Task; parentTask?: Task }) {
await this.removeClineFromStack()

// Get current global mode first
let currentGlobalMode = (await this.getState()).mode

const targetModeSlug = historyItem.lastActiveModeSlug ?? currentGlobalMode

if (targetModeSlug !== currentGlobalMode) {
await this.handleModeSwitch(targetModeSlug)
// After switching, getState() will return the new active mode and its associated configs
}

// Re-fetch state after potential mode switch to get correct apiConfig, prompts, etc.
const {
apiConfiguration,
diffEnabled: enableDiff,
Expand All @@ -571,6 +602,7 @@ export class ClineProvider
rootTask: historyItem.rootTask,
parentTask: historyItem.parentTask,
taskNumber: historyItem.number,
currentModeSlug: targetModeSlug, // Pass the determined target mode slug
onCreated: (cline) => this.emit("clineCreated", cline),
})

Expand Down Expand Up @@ -773,10 +805,13 @@ export class ClineProvider
* @param newMode The mode to switch to
*/
public async handleModeSwitch(newMode: Mode) {
const cline = this.getCurrentCline()
// Telemetry for mode switch (provider level)
const cline = this.getCurrentCline() // Get current cline for task ID if available
TelemetryService.instance.captureModeSwitch(cline?.taskId || "global", newMode)

if (cline) {
TelemetryService.instance.captureModeSwitch(cline.taskId, newMode)
await cline.updateCurrentModeSlug(newMode)
cline.emit("taskModeSwitched", cline.taskId, newMode)
}

Expand Down
Loading