From df09e55d2c01565c7f27a76ea2b7216c71d6cc97 Mon Sep 17 00:00:00 2001 From: Sean McNamara Date: Fri, 15 May 2026 13:56:30 -0400 Subject: [PATCH 1/2] feat(chat): port concurrent conversations --- packages/types/src/task.ts | 4 +- packages/types/src/vscode-extension-host.ts | 13 + src/core/task/Task.ts | 108 ++- src/core/webview/ClineProvider.ts | 563 ++++++++++---- .../ClineProvider.flicker-free-cancel.spec.ts | 75 +- .../webview/__tests__/ClineProvider.spec.ts | 14 +- src/core/webview/webviewMessageHandler.ts | 59 +- .../chat/ActiveConversationList.tsx | 216 ++++++ webview-ui/src/components/chat/ChatView.tsx | 696 ++++++++++++------ 9 files changed, 1286 insertions(+), 462 deletions(-) create mode 100644 webview-ui/src/components/chat/ActiveConversationList.tsx diff --git a/packages/types/src/task.ts b/packages/types/src/task.ts index 56a7572898..17357e0213 100644 --- a/packages/types/src/task.ts +++ b/packages/types/src/task.ts @@ -22,7 +22,7 @@ export interface TaskProviderLike { options?: CreateTaskOptions, configuration?: RooCodeSettings, ): Promise - cancelTask(): Promise + cancelTask(taskId?: string): Promise clearTask(): Promise resumeTask(taskId: string): void @@ -94,6 +94,8 @@ export interface CreateTaskOptions { consecutiveMistakeLimit?: number experiments?: Record initialTodos?: TodoItem[] + /** Whether the task should become the visible task in the UI (default: true). */ + focus?: boolean /** Initial status for the task's history item (e.g., "active" for child tasks) */ initialStatus?: "active" | "delegated" | "completed" /** Whether to start the task loop immediately (default: true). diff --git a/packages/types/src/vscode-extension-host.ts b/packages/types/src/vscode-extension-host.ts index 202326eb01..334ee5bcf5 100644 --- a/packages/types/src/vscode-extension-host.ts +++ b/packages/types/src/vscode-extension-host.ts @@ -245,6 +245,18 @@ export interface OpenAiCodexRateLimitsMessage { error?: string } +export interface ActiveConversationSummary { + rootTaskId: string + activeTaskId: string + rootTask: string + activeTask: string + ts: number + status: "running" | "interactive" | "resumable" | "idle" | "none" + parentTaskId?: string + queuedMessageCount: number + steerMessageCount: number +} + export type ExtensionState = Pick< GlobalSettings, | "currentApiConfigName" @@ -315,6 +327,7 @@ export type ExtensionState = Pick< apiConfiguration: ProviderSettings uriScheme?: string shouldShowAnnouncement: boolean + activeConversations?: ActiveConversationSummary[] taskHistory: HistoryItem[] diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 26b7295729..e19a1ff00d 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -29,6 +29,7 @@ import { type ClineMessage, type ClineSay, type ClineAsk, + type ExtensionMessage, type ToolProgressStatus, type HistoryItem, type CreateTaskOptions, @@ -523,7 +524,7 @@ export class Task extends EventEmitter implements TaskLike { this.messageQueueStateChangedHandler = () => { this.emit(RooCodeEventName.TaskUserMessage, this.taskId) this.emit(RooCodeEventName.QueuedMessagesUpdated, this.taskId, this.messageQueueService.messages) - this.providerRef.deref()?.postStateToWebviewWithoutTaskHistory() + this.postTaskStateToWebview().catch(() => undefined) } this.messageQueueService.on("stateChanged", this.messageQueueStateChangedHandler) @@ -666,9 +667,13 @@ export class Task extends EventEmitter implements TaskLike { this.providerProfileChangeListener = async () => { try { + if (!provider.isTaskVisible(this.taskId)) { + return + } const newState = await provider.getState() if (newState?.apiConfiguration) { this.updateApiConfiguration(newState.apiConfiguration) + this.setTaskApiConfigName(newState.currentApiConfigName) } } catch (error) { console.error( @@ -681,6 +686,50 @@ export class Task extends EventEmitter implements TaskLike { provider.on(RooCodeEventName.ProviderProfileChanged, this.providerProfileChangeListener) } + private async getTaskScopedState(): Promise> | undefined> { + const provider = this.providerRef.deref() + if (!provider) { + return undefined + } + + const state = await provider.getState() + + return { + ...state, + mode: await this.getTaskMode().catch(() => state.mode ?? defaultModeSlug), + apiConfiguration: this.apiConfiguration, + currentApiConfigName: await this.getTaskApiConfigName().catch(() => state.currentApiConfigName), + } + } + + private async postTaskStateToWebview(): Promise { + const provider = this.providerRef.deref() + if (!provider) { + return + } + + if (typeof provider.postTaskStateToWebview === "function") { + await provider.postTaskStateToWebview(this.taskId) + return + } + + await provider.postStateToWebviewWithoutTaskHistory?.() + } + + private async postTaskMessageToWebview(message: ExtensionMessage): Promise { + const provider = this.providerRef.deref() + if (!provider) { + return + } + + if (typeof provider.postTaskMessageToWebview === "function") { + await provider.postTaskMessageToWebview(this.taskId, message) + return + } + + await provider.postMessageToWebview?.(message) + } + /** * Wait for the task mode to be initialized before proceeding. * This method ensures that any operations depending on the task mode @@ -1155,10 +1204,9 @@ export class Task extends EventEmitter implements TaskLike { private async addToClineMessages(message: ClineMessage) { this.clineMessages.push(message) - const provider = this.providerRef.deref() // Avoid resending large, mostly-static fields (notably taskHistory) on every chat message update. // taskHistory is maintained in-memory in the webview and updated via taskHistoryItemUpdated. - await provider?.postStateToWebviewWithoutTaskHistory() + await this.postTaskStateToWebview() this.emit(RooCodeEventName.Message, { action: "created", message }) await this.saveClineMessages() @@ -1190,8 +1238,7 @@ export class Task extends EventEmitter implements TaskLike { } private async updateClineMessage(message: ClineMessage) { - const provider = this.providerRef.deref() - await provider?.postMessageToWebview({ type: "messageUpdated", clineMessage: message }) + await this.postTaskMessageToWebview({ type: "messageUpdated", clineMessage: message }) this.emit(RooCodeEventName.Message, { action: "updated", message }) // Check if we should sync to cloud and haven't already synced this message @@ -1362,7 +1409,7 @@ export class Task extends EventEmitter implements TaskLike { // Automatically approve if the ask according to the user's settings. const provider = this.providerRef.deref() - const state = provider ? await provider.getState() : undefined + const state = await this.getTaskScopedState() const approval = await checkAutoApproval({ state, ask: type, text, isProtected }) if (approval.decision === "approve") { @@ -1399,7 +1446,7 @@ export class Task extends EventEmitter implements TaskLike { if (message) { this.interactiveAsk = message this.emit(RooCodeEventName.TaskInteractive, this.taskId) - provider?.postMessageToWebview({ type: "interactionRequired" }) + this.postTaskMessageToWebview({ type: "interactionRequired" }).catch(() => undefined) } }, statusMutationTimeout), ) @@ -1608,7 +1655,7 @@ export class Task extends EventEmitter implements TaskLike { // Update this task's API configuration to match the new profile // This ensures the parser state is synchronized with the selected model - const newState = await provider.getState() + const newState = await this.getTaskScopedState() if (newState?.apiConfiguration) { this.updateApiConfiguration(newState.apiConfiguration) } @@ -1653,7 +1700,7 @@ export class Task extends EventEmitter implements TaskLike { const systemPrompt = await this.getSystemPrompt() // Get condensing configuration - const state = await this.providerRef.deref()?.getState() + const state = await this.getTaskScopedState() const customCondensingPrompt = state?.customSupportPrompts?.CONDENSE const { mode, apiConfiguration } = state ?? {} @@ -1892,7 +1939,7 @@ export class Task extends EventEmitter implements TaskLike { return { enabledToolCount: 0, enabledServerCount: 0 } } - const { mcpEnabled } = (await provider.getState()) ?? {} + const { mcpEnabled } = (await this.getTaskScopedState()) ?? {} if (!(mcpEnabled ?? true)) { return { enabledToolCount: 0, enabledServerCount: 0 } } @@ -1948,7 +1995,7 @@ export class Task extends EventEmitter implements TaskLike { // The todo list is already set in the constructor if initialTodos were provided // No need to add any messages - the todoList property is already set - await this.providerRef.deref()?.postStateToWebviewWithoutTaskHistory() + await this.postTaskStateToWebview() await this.say("text", task, images) @@ -2593,7 +2640,7 @@ export class Task extends EventEmitter implements TaskLike { ) const provider = this.providerRef.deref() - const state = provider ? await provider.getState() : undefined + const state = await this.getTaskScopedState() const showRooIgnoredFiles = state?.showRooIgnoredFiles ?? false const includeDiagnosticMessages = state?.includeDiagnosticMessages ?? true @@ -2616,7 +2663,7 @@ export class Task extends EventEmitter implements TaskLike { if (slashCommandMode) { const provider = this.providerRef.deref() if (provider) { - const state = await provider.getState() + const state = await this.getTaskScopedState() const targetMode = getModeBySlug(slashCommandMode, state?.customModes) if (targetMode) { await provider.handleModeSwitch(slashCommandMode) @@ -2671,7 +2718,7 @@ export class Task extends EventEmitter implements TaskLike { } satisfies ClineApiReqInfo) await this.saveClineMessages() - await this.providerRef.deref()?.postStateToWebviewWithoutTaskHistory() + await this.postTaskStateToWebview() try { let cacheWriteTokens = 0 @@ -3300,7 +3347,7 @@ export class Task extends EventEmitter implements TaskLike { ) // Apply exponential backoff similar to first-chunk errors when auto-resubmit is enabled - const stateForBackoff = await this.providerRef.deref()?.getState() + const stateForBackoff = await this.getTaskScopedState() if (stateForBackoff?.autoApprovalEnabled) { await this.backoffAndAnnounce(currentItem.retryAttempt ?? 0, error) @@ -3432,7 +3479,7 @@ export class Task extends EventEmitter implements TaskLike { } await this.saveClineMessages() - await this.providerRef.deref()?.postStateToWebviewWithoutTaskHistory() + await this.postTaskStateToWebview() // No legacy text-stream tool parser state to reset. @@ -3676,7 +3723,7 @@ export class Task extends EventEmitter implements TaskLike { // apiConversationHistory at line 1876. Since the assistant failed to respond, // we need to remove that message before retrying to avoid having two consecutive // user messages (which would cause tool_result validation errors). - let state = await this.providerRef.deref()?.getState() + let state = await this.getTaskScopedState() if (this.apiConversationHistory.length > 0) { const lastMessage = this.apiConversationHistory[this.apiConversationHistory.length - 1] if (lastMessage.role === "user") { @@ -3773,7 +3820,8 @@ export class Task extends EventEmitter implements TaskLike { } private async getSystemPrompt(): Promise { - const { mcpEnabled } = (await this.providerRef.deref()?.getState()) ?? {} + const state = await this.getTaskScopedState() + const { mcpEnabled } = state ?? {} let mcpHub: McpHub | undefined if (mcpEnabled ?? true) { const provider = this.providerRef.deref() @@ -3797,8 +3845,6 @@ export class Task extends EventEmitter implements TaskLike { const rooIgnoreInstructions = this.rooIgnoreController?.getInstructions() - const state = await this.providerRef.deref()?.getState() - const { mode, customModes, @@ -3857,7 +3903,7 @@ export class Task extends EventEmitter implements TaskLike { } private async handleContextWindowExceededError(): Promise { - const state = await this.providerRef.deref()?.getState() + const state = await this.getTaskScopedState() const { profileThresholds = {}, mode, apiConfiguration } = state ?? {} const { contextTokens } = this.getTokenUsage() @@ -3881,7 +3927,7 @@ export class Task extends EventEmitter implements TaskLike { `Forcing truncation to ${FORCED_CONTEXT_REDUCTION_PERCENT}% of current context.`, ) // Send condenseTaskContextStarted to show in-progress indicator - await this.providerRef.deref()?.postMessageToWebview({ type: "condenseTaskContextStarted", text: this.taskId }) + await this.postTaskMessageToWebview({ type: "condenseTaskContextStarted", text: this.taskId }) // Build tools for condensing metadata (same tools used for normal API calls) const provider = this.providerRef.deref() @@ -3975,9 +4021,7 @@ export class Task extends EventEmitter implements TaskLike { } finally { // Notify webview that context management is complete (removes in-progress spinner) // IMPORTANT: Must always be sent to dismiss the spinner, even on error - await this.providerRef - .deref() - ?.postMessageToWebview({ type: "condenseTaskContextResponse", text: this.taskId }) + await this.postTaskMessageToWebview({ type: "condenseTaskContextResponse", text: this.taskId }) } } @@ -3988,7 +4032,7 @@ export class Task extends EventEmitter implements TaskLike { * the `api_req_rate_limit_wait` say type (not an error). */ private async maybeWaitForProviderRateLimit(retryAttempt: number): Promise { - const state = await this.providerRef.deref()?.getState() + const state = await this.getTaskScopedState() const rateLimitSeconds = state?.apiConfiguration?.rateLimitSeconds ?? this.apiConfiguration?.rateLimitSeconds ?? 0 @@ -4019,7 +4063,7 @@ export class Task extends EventEmitter implements TaskLike { retryAttempt: number = 0, options: { skipProviderRateLimit?: boolean } = {}, ): ApiStream { - const state = await this.providerRef.deref()?.getState() + const state = await this.getTaskScopedState() const { apiConfiguration, @@ -4090,9 +4134,7 @@ export class Task extends EventEmitter implements TaskLike { // This notification must be sent here (not earlier) because the early check uses stale token count // (before user message is added to history), which could incorrectly skip showing the indicator if (contextManagementWillRun && autoCondenseContext) { - await this.providerRef - .deref() - ?.postMessageToWebview({ type: "condenseTaskContextStarted", text: this.taskId }) + await this.postTaskMessageToWebview({ type: "condenseTaskContextStarted", text: this.taskId }) } // Build tools for condensing metadata (same tools used for normal API calls) @@ -4212,9 +4254,7 @@ export class Task extends EventEmitter implements TaskLike { // This removes the in-progress spinner and allows the completed result to show // IMPORTANT: Must always be sent to dismiss the spinner, even on error if (contextManagementWillRun && autoCondenseContext) { - await this.providerRef - .deref() - ?.postMessageToWebview({ type: "condenseTaskContextResponse", text: this.taskId }) + await this.postTaskMessageToWebview({ type: "condenseTaskContextResponse", text: this.taskId }) } } } @@ -4409,7 +4449,7 @@ export class Task extends EventEmitter implements TaskLike { // Shared exponential backoff for retries (first-chunk and mid-stream) private async backoffAndAnnounce(retryAttempt: number, error: any): Promise { try { - const state = await this.providerRef.deref()?.getState() + const state = await this.getTaskScopedState() const baseDelay = state?.requestDelaySeconds || 5 let exponentialDelay = Math.min( diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index ad339f52a4..1985a8b665 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -36,6 +36,7 @@ import { type ToolUsage, type ExtensionMessage, type ExtensionState, + type ActiveConversationSummary, type MarketplaceInstalledMetadata, RooCodeEventName, requestyDefaultModelId, @@ -137,6 +138,8 @@ export class ClineProvider private webviewDisposables: vscode.Disposable[] = [] private view?: vscode.WebviewView | vscode.WebviewPanel private clineStack: Task[] = [] + private visibleTaskId?: string + private hasExplicitTaskSelectionClear = false private codeIndexStatusSubscription?: vscode.Disposable private codeIndexManager?: CodeIndexManager private _workspaceTracker?: WorkspaceTracker // workSpaceTracker read-only for access outside this class @@ -156,6 +159,8 @@ export class ClineProvider private static readonly GLOBAL_STATE_WRITE_THROUGH_DEBOUNCE_MS = 5000 // 5 seconds private pendingOperations: Map = new Map() private static readonly PENDING_OPERATION_TIMEOUT_MS = 30000 // 30 seconds + private activeConversationsUpdateTimer: ReturnType | null = null + private static readonly ACTIVE_CONVERSATIONS_UPDATE_DEBOUNCE_MS = 250 private cloudOrganizationsCache: CloudOrganizationMembership[] | null = null private cloudOrganizationsCacheTimestamp: number | null = null @@ -237,24 +242,40 @@ export class ClineProvider // We do something fairly similar for the IPC-based API. this.taskCreationCallback = (instance: Task) => { this.emit(RooCodeEventName.TaskCreated, instance) + const broadcastTaskState = () => { + this.postTaskStateToWebview(instance.taskId).catch((error) => { + this.log( + `[taskCreationCallback] Failed to broadcast task state for ${instance.taskId}: ${ + error instanceof Error ? error.message : String(error) + }`, + ) + }) + } // Create named listener functions so we can remove them later. - const onTaskStarted = () => this.emit(RooCodeEventName.TaskStarted, instance.taskId) + const onTaskStarted = () => { + this.emit(RooCodeEventName.TaskStarted, instance.taskId) + broadcastTaskState() + } const onTaskCompleted = (taskId: string, tokenUsage: TokenUsage, toolUsage: ToolUsage) => { this.emit(RooCodeEventName.TaskCompleted, taskId, tokenUsage, toolUsage) + broadcastTaskState() } const onTaskAborted = async () => { this.emit(RooCodeEventName.TaskAborted, instance.taskId) + broadcastTaskState() try { // Only rehydrate on genuine streaming failures. // User-initiated cancels are handled by cancelTask(). if (instance.abortReason === "streaming_failed") { - // Defensive safeguard: if another path already replaced this instance, skip - const current = this.getCurrentTask() - if (current && current.instanceId !== instance.instanceId) { + // Defensive safeguard: if another path already replaced or removed this instance, skip + const current = this.getTaskById(instance.taskId) + if (!current || current.instanceId !== instance.instanceId) { this.log( - `[onTaskAborted] Skipping rehydrate: current instance ${current.instanceId} != aborted ${instance.instanceId}`, + `[onTaskAborted] Skipping rehydrate for ${instance.taskId}: active instance ${ + current?.instanceId ?? "none" + } != aborted ${instance.instanceId}`, ) return } @@ -262,7 +283,10 @@ export class ClineProvider const { historyItem } = await this.getTaskWithId(instance.taskId) const rootTask = instance.rootTask const parentTask = instance.parentTask - await this.createTaskWithHistoryItem({ ...historyItem, rootTask, parentTask }) + await this.createTaskWithHistoryItem( + { ...historyItem, rootTask, parentTask }, + { replaceExistingTask: true, focus: this.isTaskVisible(instance.taskId) }, + ) } } catch (error) { this.log( @@ -274,12 +298,30 @@ export class ClineProvider } const onTaskFocused = () => this.emit(RooCodeEventName.TaskFocused, instance.taskId) const onTaskUnfocused = () => this.emit(RooCodeEventName.TaskUnfocused, instance.taskId) - const onTaskActive = (taskId: string) => this.emit(RooCodeEventName.TaskActive, taskId) - const onTaskInteractive = (taskId: string) => this.emit(RooCodeEventName.TaskInteractive, taskId) - const onTaskResumable = (taskId: string) => this.emit(RooCodeEventName.TaskResumable, taskId) - const onTaskIdle = (taskId: string) => this.emit(RooCodeEventName.TaskIdle, taskId) - const onTaskPaused = (taskId: string) => this.emit(RooCodeEventName.TaskPaused, taskId) - const onTaskUnpaused = (taskId: string) => this.emit(RooCodeEventName.TaskUnpaused, taskId) + const onTaskActive = (taskId: string) => { + this.emit(RooCodeEventName.TaskActive, taskId) + broadcastTaskState() + } + const onTaskInteractive = (taskId: string) => { + this.emit(RooCodeEventName.TaskInteractive, taskId) + broadcastTaskState() + } + const onTaskResumable = (taskId: string) => { + this.emit(RooCodeEventName.TaskResumable, taskId) + broadcastTaskState() + } + const onTaskIdle = (taskId: string) => { + this.emit(RooCodeEventName.TaskIdle, taskId) + broadcastTaskState() + } + const onTaskPaused = (taskId: string) => { + this.emit(RooCodeEventName.TaskPaused, taskId) + broadcastTaskState() + } + const onTaskUnpaused = (taskId: string) => { + this.emit(RooCodeEventName.TaskUnpaused, taskId) + broadcastTaskState() + } const onTaskSpawned = (taskId: string) => this.emit(RooCodeEventName.TaskSpawned, taskId) const onTaskUserMessage = (taskId: string) => this.emit(RooCodeEventName.TaskUserMessage, taskId) const onTaskTokenUsageUpdated = (taskId: string, tokenUsage: TokenUsage, toolUsage: ToolUsage) => @@ -321,6 +363,195 @@ export class ClineProvider } } + private getTaskById(taskId?: string): Task | undefined { + if (!taskId) { + return undefined + } + + return this.clineStack.find((task) => task.taskId === taskId) + } + + private getRootTaskId(task: Pick): string { + return task.rootTaskId ?? task.taskId + } + + private getTaskActivityTs(task: Task): number { + return ( + this.taskHistoryStore.get(task.taskId)?.ts ?? + this.taskHistoryStore.get(this.getRootTaskId(task))?.ts ?? + task.clineMessages.at(-1)?.ts ?? + 0 + ) + } + + public isTaskVisible(taskId: string): boolean { + return this.visibleTaskId === taskId + } + + private async syncVisibleTaskContext(task: Task): Promise { + const [mode, currentApiConfigName] = await Promise.all([ + typeof task.getTaskMode === "function" + ? task.getTaskMode().catch(() => defaultModeSlug) + : Promise.resolve(task.taskMode ?? defaultModeSlug), + typeof task.getTaskApiConfigName === "function" + ? task.getTaskApiConfigName().catch(() => undefined) + : Promise.resolve(task.taskApiConfigName), + ]) + + await this.contextProxy.setValue("mode", mode) + if (task.apiConfiguration) { + await this.contextProxy.setProviderSettings(task.apiConfiguration) + } + + if (currentApiConfigName !== undefined) { + await this.contextProxy.setValue("currentApiConfigName", currentApiConfigName) + } + + this.emit(RooCodeEventName.ModeChanged, mode) + + if (task.apiConfiguration?.apiProvider) { + this.emit(RooCodeEventName.ProviderProfileChanged, { + name: currentApiConfigName ?? "default", + provider: task.apiConfiguration.apiProvider, + }) + } + } + + private getActiveConversationSummaries(): ActiveConversationSummary[] { + const taskByRoot = new Map() + + for (const task of this.clineStack) { + const rootTaskId = this.getRootTaskId(task) + const existing = taskByRoot.get(rootTaskId) + + if (!existing || this.getTaskActivityTs(task) >= this.getTaskActivityTs(existing)) { + taskByRoot.set(rootTaskId, task) + } + } + + return Array.from(taskByRoot.values()) + .map((task) => { + const rootTaskId = this.getRootTaskId(task) + const rootTaskItem = this.taskHistoryStore.get(rootTaskId) + const activeTaskItem = this.taskHistoryStore.get(task.taskId) + const taskMetadata = task.metadata + const queuedMessages = task.queuedMessages ?? [] + const steerMessageCount = queuedMessages.filter( + (message) => (message as { deliveryMode?: string }).deliveryMode === "steer", + ).length + + return { + rootTaskId, + activeTaskId: task.taskId, + rootTask: rootTaskItem?.task ?? activeTaskItem?.task ?? taskMetadata?.task ?? rootTaskId, + activeTask: activeTaskItem?.task ?? taskMetadata?.task ?? rootTaskItem?.task ?? task.taskId, + ts: this.getTaskActivityTs(task), + status: task.taskStatus ?? "running", + parentTaskId: task.parentTaskId, + queuedMessageCount: queuedMessages.length, + steerMessageCount, + } satisfies ActiveConversationSummary + }) + .sort((a, b) => { + if (a.activeTaskId === this.visibleTaskId) { + return -1 + } + if (b.activeTaskId === this.visibleTaskId) { + return 1 + } + return b.ts - a.ts + }) + } + + private postActiveConversationsStateToWebview(): void { + const taskStateSeq = ++this.clineMessagesSeq + this.postMessageToWebview({ + type: "state", + state: { + clineMessagesSeq: taskStateSeq, + activeConversations: this.getActiveConversationSummaries(), + }, + }) + } + + private scheduleActiveConversationsStateToWebview(options: { immediate?: boolean } = {}): void { + if (this._disposed) { + return + } + + const flush = () => { + this.activeConversationsUpdateTimer = null + if (!this._disposed) { + this.postActiveConversationsStateToWebview() + } + } + + if (options.immediate) { + if (this.activeConversationsUpdateTimer) { + clearTimeout(this.activeConversationsUpdateTimer) + this.activeConversationsUpdateTimer = null + } + flush() + return + } + + if (!this.activeConversationsUpdateTimer) { + this.activeConversationsUpdateTimer = setTimeout( + flush, + ClineProvider.ACTIVE_CONVERSATIONS_UPDATE_DEBOUNCE_MS, + ) + } + } + + public async selectTask(taskId?: string, options?: { broadcast?: boolean }): Promise { + const { broadcast = true } = options ?? {} + const previousTask = this.getTaskById(this.visibleTaskId) + const nextTask = this.getTaskById(taskId) + const nextSelectionCleared = taskId === undefined && nextTask === undefined + + if (previousTask?.taskId === nextTask?.taskId && this.hasExplicitTaskSelectionClear === nextSelectionCleared) { + if (broadcast) { + if (nextTask) { + await this.syncVisibleTaskContext(nextTask) + } + await this.postStateToWebviewWithoutTaskHistory() + } + return + } + + if (previousTask) { + previousTask.emit(RooCodeEventName.TaskUnfocused) + } + + this.visibleTaskId = nextTask?.taskId + this.hasExplicitTaskSelectionClear = nextSelectionCleared + + if (nextTask) { + await this.syncVisibleTaskContext(nextTask) + nextTask.emit(RooCodeEventName.TaskFocused) + } + + if (broadcast) { + await this.postStateToWebviewWithoutTaskHistory() + } + } + + public async postTaskStateToWebview(taskId: string): Promise { + if (this.isTaskVisible(taskId)) { + await this.postStateToWebviewWithoutTaskHistory() + return + } + this.scheduleActiveConversationsStateToWebview({ immediate: true }) + } + + public async postTaskMessageToWebview(taskId: string, message: ExtensionMessage): Promise { + if (this.isTaskVisible(taskId)) { + await this.postMessageToWebview(message) + return + } + this.scheduleActiveConversationsStateToWebview() + } + /** * Initialize the TaskHistoryStore and migrate from globalState if needed. */ @@ -403,11 +634,11 @@ export class ClineProvider // The instance is pushed to the top of the stack (LIFO order). // When the task is completed, the top instance is removed, reactivating the // previous task. - async addClineToStack(task: Task) { + async addClineToStack(task: Task, options?: { focus?: boolean }) { + const { focus = true } = options ?? {} // Add this cline instance into the stack that represents the order of // all the called tasks. this.clineStack.push(task) - task.emit(RooCodeEventName.TaskFocused) // Perform special setup provider specific tasks. await this.performPreparationTasks(task) @@ -418,6 +649,12 @@ export class ClineProvider if (!state || typeof state.mode !== "string") { throw new Error(t("common:errors.retrieve_current_mode")) } + + if (focus) { + await this.selectTask(task.taskId, { broadcast: false }) + } else if (this.isViewLaunched) { + await this.postStateToWebviewWithoutClineMessages() + } } async performPreparationTasks(cline: Task) { @@ -438,23 +675,34 @@ export class ClineProvider } } - // Removes and destroys the top Cline instance (the current finished task), - // activating the previous one (resuming the parent task). - async removeClineFromStack(options?: { skipDelegationRepair?: boolean }) { + // Removes and destroys an active Task instance. + async removeClineFromStack(options?: { skipDelegationRepair?: boolean; taskId?: string; broadcast?: boolean }) { if (this.clineStack.length === 0) { return } - // Pop the top Cline instance from the stack. - let task = this.clineStack.pop() + const { broadcast = true } = options ?? {} + const taskIdToRemove = + options?.taskId ?? this.visibleTaskId ?? this.clineStack[this.clineStack.length - 1]?.taskId + const taskIndex = this.clineStack.findIndex((task) => task.taskId === taskIdToRemove) + + if (taskIndex === -1) { + return + } + + let task: Task | undefined = this.clineStack.splice(taskIndex, 1)[0] if (task) { // Capture delegation metadata before abort/dispose, since abortTask(true) // is async and the task reference is cleared afterwards. const childTaskId = task.taskId const parentTaskId = task.parentTaskId + const wasVisible = this.visibleTaskId === childTaskId - task.emit(RooCodeEventName.TaskUnfocused) + if (wasVisible) { + this.visibleTaskId = undefined + task.emit(RooCodeEventName.TaskUnfocused) + } try { // Abort the running task and set isAbandoned to true so @@ -479,7 +727,7 @@ export class ClineProvider task = undefined // Delegation-aware parent metadata repair: - // If the popped task was a delegated child, repair the parent's metadata + // If the removed task was a delegated child, repair the parent's metadata // so it transitions from "delegated" back to "active" and becomes resumable // from the task history list. // Skip when called from delegateParentAndOpenChild() during nested delegation @@ -500,7 +748,7 @@ export class ClineProvider ) } } catch (err) { - // Non-fatal: log but do not block the pop operation. + // Non-fatal: log but do not block the removal operation. this.log( `[ClineProvider#removeClineFromStack] Failed to repair parent metadata for ${parentTaskId} (non-fatal): ${ err instanceof Error ? err.message : String(err) @@ -508,6 +756,15 @@ export class ClineProvider ) } } + + if (broadcast) { + if (wasVisible) { + const fallbackTaskId = this.clineStack[this.clineStack.length - 1]?.taskId + await this.selectTask(fallbackTaskId) + } else { + await this.postStateToWebviewWithoutClineMessages() + } + } } } @@ -896,126 +1153,92 @@ export class ClineProvider public async createTaskWithHistoryItem( historyItem: HistoryItem & { rootTask?: Task; parentTask?: Task }, - options?: { startTask?: boolean }, + options?: { startTask?: boolean; replaceExistingTask?: boolean; focus?: boolean }, ) { const isCliRuntime = process.env.ROO_CLI_RUNTIME === "1" // CLI injects runtime provider settings from command flags/env at startup. // Restoring provider profiles from task history can overwrite those // runtime settings with stale/incomplete persisted profiles. const skipProfileRestoreFromHistory = isCliRuntime + const { startTask = true, replaceExistingTask = false, focus = true } = options ?? {} + const existingTask = this.getTaskById(historyItem.id) - // Check if we're rehydrating the current task to avoid flicker - const currentTask = this.getCurrentTask() - const isRehydratingCurrentTask = currentTask && currentTask.taskId === historyItem.id - - if (!isRehydratingCurrentTask) { - await this.removeClineFromStack() + if (existingTask && !replaceExistingTask) { + if (focus) { + await this.selectTask(existingTask.taskId) + } + return existingTask } - // If the history item has a saved mode, restore it and its associated API configuration. if (historyItem.mode) { - // Validate that the mode still exists const customModes = await this.customModesManager.getCustomModes() const modeExists = getModeBySlug(historyItem.mode, customModes) !== undefined if (!modeExists) { - // Mode no longer exists, fall back to default mode. this.log( `Mode '${historyItem.mode}' from history no longer exists. Falling back to default mode '${defaultModeSlug}'.`, ) historyItem.mode = defaultModeSlug } + } + + if (historyItem.apiConfigName && skipProfileRestoreFromHistory) { + this.log( + `Skipping restore of provider profile '${historyItem.apiConfigName}' for task ${historyItem.id} in CLI runtime.`, + ) + } - await this.updateGlobalState("mode", historyItem.mode) + const { apiConfiguration, enableCheckpoints, checkpointTimeout, experiments } = await this.getState() + const listApiConfig = await this.providerSettingsManager.listConfig() + const lockApiConfigAcrossModes = this.context.workspaceState.get("lockApiConfigAcrossModes", false) + await this.updateGlobalState("listApiConfigMeta", listApiConfig) - // Load the saved API config for the restored mode if it exists. - // Skip mode-based profile activation if historyItem.apiConfigName exists, - // since the task's specific provider profile will override it anyway. - const lockApiConfigAcrossModes = this.context.workspaceState.get("lockApiConfigAcrossModes", false) + let restoredApiConfiguration = apiConfiguration + let restoredApiConfigName = historyItem.apiConfigName - if (!historyItem.apiConfigName && !lockApiConfigAcrossModes && !skipProfileRestoreFromHistory) { + if (!skipProfileRestoreFromHistory) { + let profileNameToRestore = historyItem.apiConfigName + + if (!profileNameToRestore && historyItem.mode && !lockApiConfigAcrossModes) { const savedConfigId = await this.providerSettingsManager.getModeConfigId(historyItem.mode) - const listApiConfig = await this.providerSettingsManager.listConfig() - - // Update listApiConfigMeta first to ensure UI has latest data. - await this.updateGlobalState("listApiConfigMeta", listApiConfig) - - // If this mode has a saved config, use it. - if (savedConfigId) { - const profile = listApiConfig.find(({ id }) => id === savedConfigId) - - if (profile?.name) { - try { - // Check if the profile has actual API configuration (not just an id). - // In CLI mode, the ProviderSettingsManager may return empty default profiles - // that only contain 'id' and 'name' fields. Activating such a profile would - // overwrite the CLI's working API configuration with empty settings. - const fullProfile = await this.providerSettingsManager.getProfile({ name: profile.name }) - const hasActualSettings = !!fullProfile.apiProvider - - if (hasActualSettings) { - await this.activateProviderProfile({ name: profile.name }) - } else { - // The task will continue with the current/default configuration. - } - } catch (error) { - // Log the error but continue with task restoration. - this.log( - `Failed to restore API configuration for mode '${historyItem.mode}': ${ - error instanceof Error ? error.message : String(error) - }. Continuing with default configuration.`, - ) - // The task will continue with the current/default configuration. - } - } - } + profileNameToRestore = listApiConfig.find(({ id }) => id === savedConfigId)?.name } - } - // If the history item has a saved API config name (provider profile), restore it. - // This overrides any mode-based config restoration above, because the task's - // specific provider profile takes precedence over mode defaults. - if (historyItem.apiConfigName && !skipProfileRestoreFromHistory) { - const listApiConfig = await this.providerSettingsManager.listConfig() - // Keep global state/UI in sync with latest profiles for parity with mode restoration above. - await this.updateGlobalState("listApiConfigMeta", listApiConfig) - const profile = listApiConfig.find(({ name }) => name === historyItem.apiConfigName) + if (profileNameToRestore) { + const profile = listApiConfig.find(({ name }) => name === profileNameToRestore) - if (profile?.name) { - try { - await this.activateProviderProfile( - { name: profile.name }, - { persistModeConfig: false, persistTaskHistory: false }, - ) - } catch (error) { - // Log the error but continue with task restoration. + if (profile?.name) { + try { + const { + id: _profileId, + name: _profileName, + ...providerSettings + } = await this.providerSettingsManager.getProfile({ name: profile.name }) + if (providerSettings.apiProvider) { + restoredApiConfiguration = providerSettings + restoredApiConfigName = profile.name + } + } catch (error) { + this.log( + `Failed to restore API configuration '${profile.name}' for task ${historyItem.id}: ${ + error instanceof Error ? error.message : String(error) + }. Continuing with current configuration.`, + ) + } + } else if (historyItem.apiConfigName) { this.log( - `Failed to restore API configuration '${historyItem.apiConfigName}' for task: ${ - error instanceof Error ? error.message : String(error) - }. Continuing with current configuration.`, + `Provider profile '${historyItem.apiConfigName}' from history no longer exists. Using current configuration.`, ) } - } else { - // Profile no longer exists, log warning but continue - this.log( - `Provider profile '${historyItem.apiConfigName}' from history no longer exists. Using current configuration.`, - ) } - } else if (historyItem.apiConfigName && skipProfileRestoreFromHistory) { - this.log( - `Skipping restore of provider profile '${historyItem.apiConfigName}' for task ${historyItem.id} in CLI runtime.`, - ) } - const { apiConfiguration, enableCheckpoints, checkpointTimeout, experiments, cloudUserInfo, taskSyncEnabled } = - await this.getState() - const task = new Task({ provider: this, - apiConfiguration, + apiConfiguration: restoredApiConfiguration, enableCheckpoints, checkpointTimeout, - consecutiveMistakeLimit: apiConfiguration.consecutiveMistakeLimit, + consecutiveMistakeLimit: restoredApiConfiguration.consecutiveMistakeLimit, historyItem, experiments, rootTask: historyItem.rootTask, @@ -1023,17 +1246,28 @@ export class ClineProvider taskNumber: historyItem.number, workspacePath: historyItem.workspace, onCreated: this.taskCreationCallback, - startTask: options?.startTask ?? true, + startTask, // Preserve the status from the history item to avoid overwriting it when the task saves messages initialStatus: historyItem.status, }) + if (typeof task.setTaskApiConfigName === "function") { + task.setTaskApiConfigName(restoredApiConfigName) + } else { + ;(task as any)._taskApiConfigName = restoredApiConfigName + ;(task as any).taskApiConfigName = restoredApiConfigName + } + if (typeof task.getTaskMode !== "function") { + ;(task as any)._taskMode = historyItem.mode ?? defaultModeSlug + ;(task as any).taskMode = historyItem.mode ?? defaultModeSlug + } - if (isRehydratingCurrentTask) { + if (existingTask && replaceExistingTask) { // Replace the current task in-place to avoid UI flicker - const stackIndex = this.clineStack.length - 1 + const stackIndex = this.clineStack.findIndex((candidate) => candidate.taskId === existingTask.taskId) // Properly dispose of the old task to ensure garbage collection const oldTask = this.clineStack[stackIndex] + const wasVisible = this.visibleTaskId === oldTask?.taskId // Abort the old task to stop running processes and mark as abandoned try { @@ -1053,16 +1287,21 @@ export class ClineProvider // Replace the task in the stack this.clineStack[stackIndex] = task - task.emit(RooCodeEventName.TaskFocused) // Perform preparation tasks and set up event listeners await this.performPreparationTasks(task) + if (focus || wasVisible) { + await this.selectTask(task.taskId, { broadcast: false }) + } else if (this.isViewLaunched) { + await this.postStateToWebviewWithoutClineMessages() + } + this.log( `[createTaskWithHistoryItem] rehydrated task ${task.taskId}.${task.instanceId} in-place (flicker-free)`, ) } else { - await this.addClineToStack(task) + await this.addClineToStack(task, { focus }) this.log( `[createTaskWithHistoryItem] ${task.parentTask ? "child" : "parent"} task ${task.taskId}.${task.instanceId} instantiated`, @@ -1778,10 +2017,12 @@ export class ClineProvider } async showTaskWithId(id: string) { - if (id !== this.getCurrentTask()?.taskId) { - // Non-current task. + const activeTask = this.getTaskById(id) + if (activeTask) { + await this.selectTask(activeTask.taskId) + } else if (id !== this.getCurrentTask()?.taskId) { const { historyItem } = await this.getTaskWithId(id) - await this.createTaskWithHistoryItem(historyItem) // Clears existing task. + await this.createTaskWithHistoryItem(historyItem) } await this.postMessageToWebview({ type: "action", action: "chatButtonClicked" }) @@ -1811,10 +2052,14 @@ export class ClineProvider } } if (!task) { + await this.postMessageToWebview({ type: "condenseTaskContextResponse", text: taskId }) throw new Error(`Task with id ${taskId} not found in stack`) } - await task.condenseContext() - await this.postMessageToWebview({ type: "condenseTaskContextResponse", text: taskId }) + try { + await task.condenseContext() + } finally { + await this.postMessageToWebview({ type: "condenseTaskContextResponse", text: taskId }) + } } // this function deletes a task from task history, and deletes its checkpoints and delete the task folder @@ -1847,13 +2092,19 @@ export class ClineProvider await collectChildIds(id) } - // Remove from stack if any of the tasks to delete are in the current task stack - for (const taskId of allIdsToDelete) { - if (taskId === this.getCurrentTask()?.taskId) { - // Close the current task instance; delegation flows will be handled via metadata if applicable. - await this.removeClineFromStack() - break - } + // Remove any live instances for tasks being deleted, not just the currently visible one. + const liveTaskIdsToDelete = this.clineStack + .filter((task) => allIdsToDelete.includes(task.taskId)) + .map((task) => task.taskId) + const removedVisibleTask = + this.visibleTaskId !== undefined && liveTaskIdsToDelete.includes(this.visibleTaskId) + + for (const taskId of liveTaskIdsToDelete) { + await this.removeClineFromStack({ taskId, broadcast: false }) + } + + if (removedVisibleTask) { + await this.selectTask(this.clineStack[this.clineStack.length - 1]?.taskId, { broadcast: false }) } // Delete all tasks from state in one batch @@ -2219,6 +2470,7 @@ export class ClineProvider uriScheme: vscode.env.uriScheme, currentTaskId: currentTask?.taskId, currentTaskItem: currentTask?.taskId ? this.taskHistoryStore.get(currentTask.taskId) : undefined, + activeConversations: this.getActiveConversationSummaries(), clineMessages: currentTask?.clineMessages || [], currentTaskTodos: currentTask?.todoList || [], messageQueue: currentTask?.messageQueueService?.messages, @@ -2765,6 +3017,15 @@ export class ClineProvider */ public getCurrentTask(): Task | undefined { + const visibleTask = this.getTaskById(this.visibleTaskId) + if (visibleTask) { + return visibleTask + } + + if (this.hasExplicitTaskSelectionClear) { + return undefined + } + if (this.clineStack.length === 0) { return undefined } @@ -2772,6 +3033,14 @@ export class ClineProvider return this.clineStack[this.clineStack.length - 1] } + public resolveMessageTask(taskId?: string): Task | undefined { + if (taskId !== undefined && taskId !== "") { + return this.getTaskById(taskId) + } + + return this.getCurrentTask() + } + public getRecentTasks(): string[] { if (this.recentTasksCache) { return this.recentTasksCache @@ -2873,19 +3142,13 @@ export class ClineProvider const { apiConfiguration, organizationAllowList, enableCheckpoints, checkpointTimeout, experiments } = await this.getState() - // Single-open-task invariant: always enforce for user-initiated top-level tasks - if (!parentTask) { - try { - await this.removeClineFromStack() - } catch { - // Non-fatal - } - } - if (!ProfileValidator.isProfileAllowed(apiConfiguration, organizationAllowList)) { throw new OrganizationAllowListViolationError(t("common:errors.violated_organization_allowlist")) } + const resolvedParentTask = parentTask as Task | undefined + const rootTask = resolvedParentTask ? (resolvedParentTask.rootTask ?? resolvedParentTask) : undefined + const task = new Task({ provider: this, apiConfiguration, @@ -2895,9 +3158,9 @@ export class ClineProvider task: text, images, experiments, - rootTask: this.clineStack.length > 0 ? this.clineStack[0] : undefined, - parentTask, - taskNumber: this.clineStack.length + 1, + rootTask, + parentTask: resolvedParentTask, + taskNumber: this.taskHistoryStore.getAll().length + 1, onCreated: this.taskCreationCallback, initialTodos: options.initialTodos, // Ensure this task is present in clineStack before startTask() emits @@ -2906,7 +3169,7 @@ export class ClineProvider ...options, }) - await this.addClineToStack(task) + await this.addClineToStack(task, { focus: options.focus ?? true }) task.start() this.log( @@ -2916,14 +3179,15 @@ export class ClineProvider return task } - public async cancelTask(): Promise { - const task = this.getCurrentTask() + public async cancelTask(taskId?: string): Promise { + const task = this.resolveMessageTask(taskId) if (!task) { return } console.log(`[cancelTask] cancelling task ${task.taskId}.${task.instanceId}`) + const wasVisible = this.isTaskVisible(task.taskId) let historyItem: HistoryItem | undefined try { @@ -2962,13 +3226,13 @@ export class ClineProvider await pWaitFor( () => - this.getCurrentTask()! === undefined || - this.getCurrentTask()!.isStreaming === false || - this.getCurrentTask()!.didFinishAbortingStream || + this.getTaskById(task.taskId) === undefined || + task.isStreaming === false || + task.didFinishAbortingStream || // If only the first chunk is processed, then there's no // need to wait for graceful abort (closes edits, browser, // etc). - this.getCurrentTask()!.isWaitingForFirstChunk, + task.isWaitingForFirstChunk, { timeout: 3_000, }, @@ -2977,7 +3241,7 @@ export class ClineProvider }) // Defensive safeguard: if current instance already changed, skip rehydrate - const current = this.getCurrentTask() + const current = this.getTaskById(task.taskId) if (current && current.instanceId !== originalInstanceId) { this.log( `[cancelTask] Skipping rehydrate: current instance ${current.instanceId} != original ${originalInstanceId}`, @@ -2987,7 +3251,7 @@ export class ClineProvider // Final race check before rehydrate to avoid duplicate rehydration { - const currentAfterCheck = this.getCurrentTask() + const currentAfterCheck = this.getTaskById(task.taskId) if (currentAfterCheck && currentAfterCheck.instanceId !== originalInstanceId) { this.log( `[cancelTask] Skipping rehydrate after final check: current instance ${currentAfterCheck.instanceId} != original ${originalInstanceId}`, @@ -3001,16 +3265,19 @@ export class ClineProvider } // Clears task again, so we need to abortTask manually above. - await this.createTaskWithHistoryItem({ ...historyItem, rootTask, parentTask }) + await this.createTaskWithHistoryItem( + { ...historyItem, rootTask, parentTask }, + { replaceExistingTask: true, focus: wasVisible }, + ) } // Clear the current task without treating it as a subtask. // This is used when the user cancels a task that is not a subtask. public async clearTask(): Promise { - if (this.clineStack.length > 0) { - const task = this.clineStack[this.clineStack.length - 1] + const task = this.getCurrentTask() + if (task) { console.log(`[clearTask] clearing task ${task.taskId}.${task.instanceId}`) - await this.removeClineFromStack() + await this.selectTask(undefined) } } diff --git a/src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts b/src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts index 4bb01347a3..434c751779 100644 --- a/src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts @@ -173,7 +173,15 @@ describe("ClineProvider flicker-free cancel", () => { instanceId: "instance-1", emit: vi.fn(), abortTask: vi.fn().mockResolvedValue(undefined), + cancelCurrentRequest: vi.fn(), + cancelAutoApprovalTimeout: vi.fn(), + supersedePendingAsk: vi.fn(), + messageQueueService: { messages: [] }, + terminalProcess: { abort: vi.fn() }, + rootTask: undefined, + parentTask: undefined, abandoned: false, + abort: false, dispose: vi.fn(), on: vi.fn(), off: vi.fn(), @@ -188,10 +196,12 @@ describe("ClineProvider flicker-free cancel", () => { } // Mock Task constructor - vi.mocked(Task).mockImplementation(() => mockTask2 as any) + vi.mocked(Task).mockImplementation(function () { + return mockTask2 as any + }) }) - it("should not remove current task from stack when rehydrating same taskId", async () => { + it("should reuse the current task instance when reopening the same taskId without replacement", async () => { // Setup: Add a task to the stack first ;(provider as any).clineStack = [mockTask1] @@ -215,30 +225,33 @@ describe("ClineProvider flicker-free cancel", () => { workspace: "/test/workspace", } - // Act: Create task with history item (should rehydrate in-place) - await provider.createTaskWithHistoryItem(historyItem) + // Act: Reopen the same task ID without forcing replacement + const task = await provider.createTaskWithHistoryItem(historyItem) // Assert: removeClineFromStack should NOT be called expect(removeClineFromStackSpy).not.toHaveBeenCalled() - // Verify the task was replaced in-place + // Verify the existing task instance was reused + expect(task).toBe(mockTask1) expect((provider as any).clineStack).toHaveLength(1) - expect((provider as any).clineStack[0]).toBe(mockTask2) + expect((provider as any).clineStack[0]).toBe(mockTask1) - // Verify old event listeners were cleaned up - expect(mockCleanupFunctions[0]).toHaveBeenCalled() - expect(mockCleanupFunctions[1]).toHaveBeenCalled() + // No replacement means no cleanup or fresh Task construction + expect(mockCleanupFunctions[0]).not.toHaveBeenCalled() + expect(mockCleanupFunctions[1]).not.toHaveBeenCalled() + expect(Task).not.toHaveBeenCalled() - // Verify new task received focus event - expect(mockTask2.emit).toHaveBeenCalledWith("taskFocused") + // Verify the existing task received focus + expect(mockTask1.emit).toHaveBeenCalledWith("taskFocused") }) - it("should remove task from stack when creating different task", async () => { + it("should append a different task without removing the existing stack", async () => { // Setup: Add a task to the stack first ;(provider as any).clineStack = [mockTask1] - // Spy on removeClineFromStack to verify it IS called + // Spy on removeClineFromStack to verify it is NOT called const removeClineFromStackSpy = vi.spyOn(provider, "removeClineFromStack").mockResolvedValue(undefined) + mockTask2.taskId = "task-2" // Create history item with different taskId const historyItem: HistoryItem = { @@ -253,13 +266,17 @@ describe("ClineProvider flicker-free cancel", () => { } // Act: Create task with different history item - await provider.createTaskWithHistoryItem(historyItem) + const task = await provider.createTaskWithHistoryItem(historyItem) - // Assert: removeClineFromStack should be called - expect(removeClineFromStackSpy).toHaveBeenCalled() + // Assert: multi-conversation restore keeps both tasks alive + expect(removeClineFromStackSpy).not.toHaveBeenCalled() + expect(task).toBe(mockTask2) + expect((provider as any).clineStack).toHaveLength(2) + expect((provider as any).clineStack[0]).toBe(mockTask1) + expect((provider as any).clineStack[1]).toBe(mockTask2) }) - it("should handle empty stack gracefully during rehydration attempt", async () => { + it("should handle empty stack gracefully during task creation", async () => { // Setup: Empty stack ;(provider as any).clineStack = [] @@ -278,14 +295,17 @@ describe("ClineProvider flicker-free cancel", () => { workspace: "/test/workspace", } - // Act: Should not error and should call removeClineFromStack - await provider.createTaskWithHistoryItem(historyItem) + // Act: Should create a fresh task without trying to remove anything + const task = await provider.createTaskWithHistoryItem(historyItem) - // Assert: removeClineFromStack should be called (no current task to rehydrate) - expect(removeClineFromStackSpy).toHaveBeenCalled() + // Assert: removeClineFromStack should not be called + expect(removeClineFromStackSpy).not.toHaveBeenCalled() + expect(task).toBe(mockTask2) + expect((provider as any).clineStack).toHaveLength(1) + expect((provider as any).clineStack[0]).toBe(mockTask2) }) - it("should maintain task stack integrity during flicker-free replacement", async () => { + it("should replace the matching task in-place when replacement is explicitly requested", async () => { // Setup: Stack with multiple tasks const mockParentTask = { taskId: "parent-task", @@ -295,9 +315,10 @@ describe("ClineProvider flicker-free cancel", () => { ;(provider as any).clineStack = [mockParentTask, mockTask1] ;(provider as any).taskEventListeners = new WeakMap() - ;(provider as any).taskEventListeners.set(mockTask1, [vi.fn()]) + const cleanup = vi.fn() + ;(provider as any).taskEventListeners.set(mockTask1, [cleanup]) - // Act: Rehydrate the current (top) task + // Act: Rehydrate the current (top) task with explicit replacement const historyItem: HistoryItem = { id: "task-1", number: 1, @@ -309,11 +330,15 @@ describe("ClineProvider flicker-free cancel", () => { workspace: "/test/workspace", } - await provider.createTaskWithHistoryItem(historyItem) + const removeClineFromStackSpy = vi.spyOn(provider, "removeClineFromStack").mockResolvedValue(undefined) + await provider.createTaskWithHistoryItem(historyItem, { replaceExistingTask: true }) // Assert: Stack should maintain parent task and replace current task + expect(removeClineFromStackSpy).not.toHaveBeenCalled() expect((provider as any).clineStack).toHaveLength(2) expect((provider as any).clineStack[0]).toBe(mockParentTask) expect((provider as any).clineStack[1]).toBe(mockTask2) + expect(mockTask1.abortTask).toHaveBeenCalledWith(true) + expect(cleanup).toHaveBeenCalled() }) }) diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index d25c6971d2..a21d477a66 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -1743,6 +1743,7 @@ describe("ClineProvider", () => { getModeConfigId: vi.fn().mockResolvedValue(undefined), listConfig: vi.fn().mockResolvedValue([]), } + const logSpy = vi.spyOn(provider, "log") // Create history item without mode const historyItem = { @@ -1756,11 +1757,12 @@ describe("ClineProvider", () => { totalCost: 0, } - // Initialize with history item - await provider.createTaskWithHistoryItem(historyItem) + // Initialize with history item without forcing a visible-task sync. + await provider.createTaskWithHistoryItem(historyItem, { focus: false }) - // Verify no mode validation occurred (mode update not called) - expect(mockContext.globalState.update).not.toHaveBeenCalledWith("mode", expect.any(String)) + // Verify no mode validation/fallback occurred. + expect(logSpy).not.toHaveBeenCalledWith(expect.stringContaining("Falling back to default mode")) + expect(historyItem).not.toHaveProperty("mode") }) test("continues with task restoration even if mode config loading fails", async () => { @@ -1788,7 +1790,7 @@ describe("ClineProvider", () => { listConfig: vi .fn() .mockResolvedValue([{ name: "test-config", id: "config-id", apiProvider: "anthropic" }]), - activateProfile: vi.fn().mockRejectedValue(new Error("Failed to load config")), + getProfile: vi.fn().mockRejectedValue(new Error("Failed to load config")), } // Spy on log method @@ -1811,7 +1813,7 @@ describe("ClineProvider", () => { // Verify error was logged but task restoration continued expect(logSpy).toHaveBeenCalledWith( - expect.stringContaining("Failed to restore API configuration for mode 'code'"), + expect.stringContaining("Failed to restore API configuration 'test-config' for task test-id"), ) }) }) diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index dc029cb7dd..e9ce823e5f 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -196,6 +196,21 @@ export const webviewMessageHandler = async ( }) return resolved } + + const restoreDroppedInput = async (text?: string, images?: string[]) => { + const restoredText = text ?? "" + const restoredImages = images ?? [] + if (!restoredText && restoredImages.length === 0) { + return + } + + await provider.postMessageToWebview({ + type: "invoke", + invoke: "setChatBoxMessage", + text: restoredText, + images: restoredImages, + }) + } /** * Shared utility to find message indices based on timestamp. * When multiple messages share the same timestamp (e.g., after condense), @@ -653,10 +668,25 @@ export const webviewMessageHandler = async ( case "askResponse": { + const targetTask = provider.resolveMessageTask(message.taskId) + if (!targetTask) { + provider.log( + `[askResponse] dropped because target task ${message.taskId ?? "(current)"} is not active`, + ) + await restoreDroppedInput(message.text, message.images) + break + } + const expectedTaskId = targetTask.taskId const resolved = await resolveIncomingImages({ text: message.text, images: message.images }) - provider - .getCurrentTask() - ?.handleWebviewAskResponse(message.askResponse!, resolved.text, resolved.images) + const stillActive = provider.resolveMessageTask(expectedTaskId) + if (!stillActive || stillActive.taskId !== expectedTaskId) { + provider.log( + `[askResponse] dropped after image resolution because task ${expectedTaskId} is not active`, + ) + await restoreDroppedInput(resolved.text, resolved.images) + break + } + stillActive.handleWebviewAskResponse(message.askResponse!, resolved.text, resolved.images) } break @@ -760,7 +790,7 @@ export const webviewMessageHandler = async ( case "terminalOperation": if (message.terminalOperation) { - provider.getCurrentTask()?.handleTerminalOperation(message.terminalOperation) + provider.resolveMessageTask(message.taskId)?.handleTerminalOperation(message.terminalOperation) } break case "clearTask": @@ -803,7 +833,13 @@ export const webviewMessageHandler = async ( provider.showTaskWithId(message.text!) break case "condenseTaskContextRequest": - provider.condenseTaskContext(message.text!) + provider.condenseTaskContext(message.text!).catch((error) => { + provider.log( + `[condenseTaskContextRequest] failed for task ${message.text}: ${ + error instanceof Error ? error.message : String(error) + }`, + ) + }) break case "deleteTaskWithId": provider.deleteTaskWithId(message.text!) @@ -1263,7 +1299,7 @@ export const webviewMessageHandler = async ( break } case "cancelTask": - await provider.cancelTask() + await provider.cancelTask(message.taskId) break case "cancelAutoApproval": // Cancel any pending auto-approval timeout for the current task @@ -3209,17 +3245,22 @@ export const webviewMessageHandler = async ( case "queueMessage": { const resolved = await resolveIncomingImages({ text: message.text, images: message.images }) - provider.getCurrentTask()?.messageQueueService.addMessage(resolved.text, resolved.images) + const targetTask = provider.resolveMessageTask(message.taskId) + if (!targetTask) { + await restoreDroppedInput(resolved.text, resolved.images) + break + } + targetTask.messageQueueService.addMessage(resolved.text, resolved.images) break } case "removeQueuedMessage": { - provider.getCurrentTask()?.messageQueueService.removeMessage(message.text ?? "") + provider.resolveMessageTask(message.taskId)?.messageQueueService.removeMessage(message.text ?? "") break } case "editQueuedMessage": { if (message.payload) { const { id, text, images } = message.payload as EditQueuedMessagePayload - provider.getCurrentTask()?.messageQueueService.updateMessage(id, text, images) + provider.resolveMessageTask(message.taskId)?.messageQueueService.updateMessage(id, text, images) } break diff --git a/webview-ui/src/components/chat/ActiveConversationList.tsx b/webview-ui/src/components/chat/ActiveConversationList.tsx new file mode 100644 index 0000000000..2ced82ef70 --- /dev/null +++ b/webview-ui/src/components/chat/ActiveConversationList.tsx @@ -0,0 +1,216 @@ +import { useEffect, useMemo, useState } from "react" +import type { ActiveConversationSummary } from "@roo-code/types" +import { Trash2 } from "lucide-react" + +import { Button, StandardTooltip } from "@src/components/ui" +import { cn } from "@src/lib/utils" + +export interface ConversationListItem extends ActiveConversationSummary { + kind: "task" | "draft" +} + +interface ActiveConversationListProps { + conversations: ConversationListItem[] + selectedConversationId?: string + onCreateConversation: () => void + onSelectConversation: (conversation: ConversationListItem) => void + onDeleteConversation: (conversation: ConversationListItem) => void +} + +const STATUS_LABELS: Record = { + running: "Running", + interactive: "Waiting", + resumable: "Paused", + idle: "Idle", + none: "Ready", +} + +export default function ActiveConversationList({ + conversations, + selectedConversationId, + onCreateConversation, + onSelectConversation, + onDeleteConversation, +}: ActiveConversationListProps) { + const [contextMenu, setContextMenu] = useState<{ + conversation: ConversationListItem + x: number + y: number + } | null>(null) + + useEffect(() => { + if (!contextMenu) { + return + } + + const handleKeyDown = (event: KeyboardEvent) => { + if (event.key === "Escape") { + setContextMenu(null) + } + } + + window.addEventListener("keydown", handleKeyDown) + return () => window.removeEventListener("keydown", handleKeyDown) + }, [contextMenu]) + + const contextMenuPosition = useMemo(() => { + if (!contextMenu) { + return undefined + } + + const width = 176 + const height = 52 + const maxLeft = typeof window !== "undefined" ? Math.max(8, window.innerWidth - width - 8) : contextMenu.x + const maxTop = typeof window !== "undefined" ? Math.max(8, window.innerHeight - height - 8) : contextMenu.y + + return { + left: Math.min(contextMenu.x, maxLeft), + top: Math.min(contextMenu.y, maxTop), + } + }, [contextMenu]) + + if (conversations.length === 0) { + return null + } + + return ( + + ) +} diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index 7026d093f5..039968aeef 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -10,7 +10,15 @@ import { appendImages } from "@src/utils/imageUtils" import { getCostBreakdownIfNeeded } from "@src/utils/costFormatting" import { batchConsecutive } from "@src/utils/batchConsecutive" -import type { ClineAsk, ClineSayTool, ClineMessage, ExtensionMessage, AudioType } from "@roo-code/types" +import type { + ClineAsk, + ClineSayTool, + ClineMessage, + ExtensionMessage, + AudioType, + TodoItem, + QueuedMessage, +} from "@roo-code/types" import { isRetiredProvider } from "@roo-code/types" import { findLast } from "@roo/array" @@ -34,6 +42,7 @@ import TelemetryBanner from "../common/TelemetryBanner" import VersionIndicator from "../common/VersionIndicator" import HistoryPreview from "../history/HistoryPreview" import Announcement from "./Announcement" +import ActiveConversationList, { type ConversationListItem } from "./ActiveConversationList" import ChatRow from "./ChatRow" import WarningRow from "./WarningRow" import { ChatTextArea } from "./ChatTextArea" @@ -59,6 +68,16 @@ export const MAX_IMAGES_PER_MESSAGE = 20 // This is the Anthropic limit. const isMac = navigator.platform.toUpperCase().indexOf("MAC") >= 0 +interface DraftConversation { + id: string + ts: number + title: string +} + +const EMPTY_MESSAGES: ClineMessage[] = [] +const EMPTY_TODOS: TodoItem[] = [] +const EMPTY_MESSAGE_QUEUE: QueuedMessage[] = [] + const ChatViewComponent: React.ForwardRefRenderFunction = ( { isHidden, showAnnouncement, hideAnnouncement }, ref, @@ -71,9 +90,11 @@ const ChatViewComponent: React.ForwardRefRenderFunction([]) + const [selectedDraftId, setSelectedDraftId] = useState(undefined) + const isDraftSelected = selectedDraftId !== undefined + const messages = useMemo( + () => (isDraftSelected ? EMPTY_MESSAGES : contextMessages), + [contextMessages, isDraftSelected], + ) + const currentTaskId = isDraftSelected ? undefined : contextCurrentTaskId + const currentTaskItem = isDraftSelected ? undefined : contextCurrentTaskItem + const currentTaskTodos = useMemo( + () => (isDraftSelected ? EMPTY_TODOS : contextCurrentTaskTodos), + [contextCurrentTaskTodos, isDraftSelected], + ) + const visibleMessageQueue = useMemo( + () => (isDraftSelected ? EMPTY_MESSAGE_QUEUE : messageQueue), + [messageQueue, isDraftSelected], + ) + + const visibleConversationIds = useMemo( + () => + new Set( + activeConversations.flatMap((conversation) => [conversation.rootTaskId, conversation.activeTaskId]), + ), + [activeConversations], + ) + + useEffect(() => { + setDraftConversations((prev) => { + const next = prev.filter((draft) => !visibleConversationIds.has(draft.id)) + return next.length === prev.length ? prev : next + }) + + if (selectedDraftId && visibleConversationIds.has(selectedDraftId)) { + setSelectedDraftId(undefined) + } + }, [selectedDraftId, visibleConversationIds]) + + const selectedConversationId = selectedDraftId ?? currentTaskId + + const conversations = useMemo(() => { + const draftItems = draftConversations + .filter((draft) => !visibleConversationIds.has(draft.id)) + .map( + (draft) => + ({ + kind: "draft", + rootTaskId: draft.id, + activeTaskId: draft.id, + rootTask: draft.title, + activeTask: draft.title, + ts: draft.ts, + status: "none", + queuedMessageCount: 0, + steerMessageCount: 0, + }) satisfies ConversationListItem, + ) + + const taskItems = activeConversations.map( + (conversation) => + ({ + ...conversation, + kind: "task", + }) satisfies ConversationListItem, + ) + + return [...draftItems, ...taskItems].sort((a, b) => { + if (a.activeTaskId === selectedConversationId) { + return -1 + } + if (b.activeTaskId === selectedConversationId) { + return 1 + } + return b.ts - a.ts + }) + }, [activeConversations, draftConversations, selectedConversationId, visibleConversationIds]) + + const hasConversationSidebar = conversations.length > 0 // Show a WarningRow when the user sends a message with a retired provider. const [showRetiredProviderWarning, setShowRetiredProviderWarning] = useState(false) @@ -364,7 +462,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction 0 || + visibleMessageQueue.length > 0 || clineAskRef.current === "command_output" ) { try { console.log("queueMessage", text, images) - vscode.postMessage({ type: "queueMessage", text, images }) + vscode.postMessage({ type: "queueMessage", taskId: currentTaskId, text, images }) setInputValue("") setSelectedImages([]) } catch (error) { @@ -621,7 +719,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction { + const draftId = `draft-${ + typeof crypto !== "undefined" && typeof crypto.randomUUID === "function" + ? crypto.randomUUID() + : Date.now().toString(36) + }` setShowRetiredProviderWarning(false) + setInputValue("") + setSelectedImages([]) + setSelectedDraftId(draftId) + setDraftConversations((prev) => [{ id: draftId, ts: Date.now(), title: "New conversation" }, ...prev]) vscode.postMessage({ type: "clearTask" }) }, []) + const handleSelectConversation = useCallback( + (conversation: ConversationListItem) => { + if (conversation.kind === "draft") { + setSelectedDraftId(conversation.activeTaskId) + if (currentTaskId) { + vscode.postMessage({ type: "clearTask" }) + } + return + } + + setSelectedDraftId(undefined) + vscode.postMessage({ type: "showTaskWithId", text: conversation.activeTaskId }) + }, + [currentTaskId], + ) + + const handleDeleteConversation = useCallback((conversation: ConversationListItem) => { + if (conversation.kind === "draft") { + setDraftConversations((prev) => prev.filter((draft) => draft.id !== conversation.activeTaskId)) + setSelectedDraftId((prev) => (prev === conversation.activeTaskId ? undefined : prev)) + return + } + + vscode.postMessage({ type: "deleteTaskWithId", text: conversation.rootTaskId }) + }, []) + // Handle stop button click from textarea const handleStopTask = useCallback(() => { - vscode.postMessage({ type: "cancelTask" }) + vscode.postMessage({ type: "cancelTask", taskId: currentTaskId }) setDidClickCancel(true) - }, [setDidClickCancel]) + }, [currentTaskId, setDidClickCancel]) // Handle enqueue button click from textarea const handleEnqueueCurrentMessage = useCallback(() => { @@ -698,13 +840,14 @@ const ChatViewComponent: React.ForwardRefRenderFunction 0) { vscode.postMessage({ type: "queueMessage", + taskId: currentTaskId, text, images: selectedImages, }) setInputValue("") setSelectedImages([]) } - }, [inputValue, selectedImages]) + }, [currentTaskId, inputValue, selectedImages]) // This logic depends on the useEffect[messages] above to set clineAsk, // after which buttons are shown and we then send an askResponse to the @@ -726,6 +869,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction 0)) { vscode.postMessage({ type: "askResponse", + taskId: currentTaskId, askResponse: "yesButtonClicked", text: trimmedInput, images: images, @@ -734,7 +878,11 @@ const ChatViewComponent: React.ForwardRefRenderFunction 0)) { vscode.postMessage({ type: "askResponse", + taskId: currentTaskId, askResponse: "yesButtonClicked", text: trimmedInput, images: images, @@ -760,7 +909,11 @@ const ChatViewComponent: React.ForwardRefRenderFunction 0)) { vscode.postMessage({ type: "askResponse", + taskId: currentTaskId, askResponse: "noButtonClicked", text: trimmedInput, images: images, @@ -818,18 +976,26 @@ const ChatViewComponent: React.ForwardRefRenderFunction { - // Handle batch file response, e.g., for file uploads - vscode.postMessage({ type: "askResponse", askResponse: "objectResponse", text: JSON.stringify(response) }) - }, []) + const handleBatchFileResponse = useCallback( + (response: { [key: string]: boolean }) => { + // Handle batch file response, e.g., for file uploads + vscode.postMessage({ + type: "askResponse", + taskId: currentTaskId, + askResponse: "objectResponse", + text: JSON.stringify(response), + }) + }, + [currentTaskId], + ) // Cancel backend auto-approval timeout when FollowUpSuggest's countdown effect cleans up. // This is called when auto-approve is toggled off, a suggestion is clicked, or the component unmounts. @@ -1525,7 +1699,12 @@ const ChatViewComponent: React.ForwardRefRenderFunction )} - {task ? ( - <> - 0 - ) - } - parentTaskId={currentTaskItem?.parentTaskId} - costBreakdown={ - currentTaskItem?.id && aggregatedCostsMap.has(currentTaskItem.id) - ? getCostBreakdownIfNeeded(aggregatedCostsMap.get(currentTaskItem.id)!, { - own: t("common:costs.own"), - subtasks: t("common:costs.subtasks"), - }) - : undefined - } - contextTokens={apiMetrics.contextTokens} - buttonsDisabled={sendingDisabled} - handleCondenseContext={handleCondenseContext} - todos={latestTodos} +
+ {hasConversationSidebar && ( + - - {checkpointWarning && ( -
- + )} +
+ {task ? ( + <> + 0 + ) + } + parentTaskId={currentTaskItem?.parentTaskId} + costBreakdown={ + currentTaskItem?.id && aggregatedCostsMap.has(currentTaskItem.id) + ? getCostBreakdownIfNeeded(aggregatedCostsMap.get(currentTaskItem.id)!, { + own: t("common:costs.own"), + subtasks: t("common:costs.subtasks"), + }) + : undefined + } + contextTokens={apiMetrics.contextTokens} + buttonsDisabled={sendingDisabled} + handleCondenseContext={handleCondenseContext} + todos={latestTodos} + /> + + {checkpointWarning && ( +
+ +
+ )} + + ) : ( +
+
+ setShowAnnouncementModal(true)} + className="absolute top-2 right-3 z-10" + /> +
+ + + {/* Everyone should see their task history if any */} + {taskHistory.length > 0 && } +
+
)} - - ) : ( -
-
- setShowAnnouncementModal(true)} - className="absolute top-2 right-3 z-10" - /> -
- - - {/* Everyone should see their task history if any */} - {taskHistory.length > 0 && } -
-
-
- )} - {!task && showWorktreesInHomeScreen && } - - {task && ( - <> -
- -
- - {areButtonsVisible && ( -
- {showScrollToBottom ? ( - <> - - - - {hasLatestCheckpoint && ( - - - - )} - - ) : ( - <> - {primaryButtonText && ( - } + + {task && ( + <> +
+ +
+ + {areButtonsVisible && ( +
+ {showScrollToBottom ? ( + <> + + + + {hasLatestCheckpoint && ( + + + + )} + + ) : ( + <> + {primaryButtonText && ( + - - + t("chat:resumeTask.title") + ? t("chat:resumeTask.tooltip") + : primaryButtonText === + t("chat:proceedAnyways.title") + ? t("chat:proceedAnyways.tooltip") + : primaryButtonText === + t( + "chat:proceedWhileRunning.title", + ) + ? t( + "chat:proceedWhileRunning.tooltip", + ) + : undefined + }> + + + )} + {secondaryButtonText && ( + + + + )} + )} - {secondaryButtonText && ( - - - - )} - +
)} -
+ )} - - )} - { - if (messageQueue[index]) { - vscode.postMessage({ type: "removeQueuedMessage", text: messageQueue[index].id }) - } - }} - onUpdate={(index, newText) => { - if (messageQueue[index]) { - vscode.postMessage({ - type: "editQueuedMessage", - payload: { id: messageQueue[index].id, text: newText, images: messageQueue[index].images }, - }) - } - }} - /> - {showRetiredProviderWarning && ( -
- vscode.postMessage({ type: "switchTab", tab: "settings" })} + { + if (visibleMessageQueue[index]) { + vscode.postMessage({ + type: "removeQueuedMessage", + text: visibleMessageQueue[index].id, + taskId: currentTaskId, + }) + } + }} + onUpdate={(index, newText) => { + if (visibleMessageQueue[index]) { + vscode.postMessage({ + type: "editQueuedMessage", + taskId: currentTaskId, + payload: { + id: visibleMessageQueue[index].id, + text: newText, + images: visibleMessageQueue[index].images, + }, + }) + } + }} + /> + {showRetiredProviderWarning && ( +
+ vscode.postMessage({ type: "switchTab", tab: "settings" })} + /> +
+ )} + handleSendMessage(inputValue, selectedImages)} + onSelectImages={selectImages} + shouldDisableImages={shouldDisableImages} + onHeightChange={() => { + if (isAtBottomRef.current && scrollPhaseRef.current !== "USER_BROWSING_HISTORY") { + scrollToBottomAuto() + } + }} + mode={mode} + setMode={setMode} + modeShortcutText={modeShortcutText} + isStreaming={isStreaming} + onStop={handleStopTask} + onEnqueueMessage={handleEnqueueCurrentMessage} /> -
- )} - handleSendMessage(inputValue, selectedImages)} - onSelectImages={selectImages} - shouldDisableImages={shouldDisableImages} - onHeightChange={() => { - if (isAtBottomRef.current && scrollPhaseRef.current !== "USER_BROWSING_HISTORY") { - scrollToBottomAuto() - } - }} - mode={mode} - setMode={setMode} - modeShortcutText={modeShortcutText} - isStreaming={isStreaming} - onStop={handleStopTask} - onEnqueueMessage={handleEnqueueCurrentMessage} - /> - - {isProfileDisabled && ( -
- -
- )} -
+ {isProfileDisabled && ( +
+ +
+ )} + +
+
+
) } From 86d7f7f103ac4630396eb93ed4d060453638beac Mon Sep 17 00:00:00 2001 From: Sean McNamara Date: Fri, 15 May 2026 14:54:09 -0400 Subject: [PATCH 2/2] feat(chat): add queue steer delivery --- packages/types/src/message.ts | 7 + packages/types/src/vscode-extension-host.ts | 3 +- .../presentAssistantMessage.ts | 16 + src/core/message-queue/MessageQueueService.ts | 188 ++++++++++-- .../__tests__/MessageQueueService.spec.ts | 37 +++ src/core/task/Task.ts | 287 +++++++++++++----- src/core/task/__tests__/Task.spec.ts | 138 +++++---- .../ask-queued-message-drain.spec.ts | 36 +++ src/core/webview/ClineProvider.ts | 27 +- .../ClineProvider.flicker-free-cancel.spec.ts | 22 ++ src/core/webview/webviewMessageHandler.ts | 18 +- webview-ui/src/components/chat/ChatView.tsx | 47 +-- .../src/components/chat/QueuedMessages.tsx | 229 +++++++++----- .../ChatView.notification-sound.spec.tsx | 13 +- .../ChatView.preserve-images.spec.tsx | 13 +- .../chat/__tests__/ChatView.spec.tsx | 60 ++-- .../chat/__tests__/QueuedMessages.spec.tsx | 55 ++++ 17 files changed, 917 insertions(+), 279 deletions(-) create mode 100644 src/core/message-queue/__tests__/MessageQueueService.spec.ts create mode 100644 webview-ui/src/components/chat/__tests__/QueuedMessages.spec.tsx diff --git a/packages/types/src/message.ts b/packages/types/src/message.ts index e518972a1c..4c89c23206 100644 --- a/packages/types/src/message.ts +++ b/packages/types/src/message.ts @@ -294,11 +294,18 @@ export type TokenUsage = z.infer * QueuedMessage */ +export const queuedMessageDeliveryModeSchema = z.enum(["queue", "steer"]) + +export type QueuedMessageDeliveryMode = z.infer + export const queuedMessageSchema = z.object({ timestamp: z.number(), + createdAt: z.number(), + updatedAt: z.number(), id: z.string(), text: z.string(), images: z.array(z.string()).optional(), + deliveryMode: queuedMessageDeliveryModeSchema.default("queue"), }) export type QueuedMessage = z.infer diff --git a/packages/types/src/vscode-extension-host.ts b/packages/types/src/vscode-extension-host.ts index 334ee5bcf5..a5b544dc52 100644 --- a/packages/types/src/vscode-extension-host.ts +++ b/packages/types/src/vscode-extension-host.ts @@ -424,7 +424,7 @@ export interface UpdateTodoListPayload { todos: any[] } -export type EditQueuedMessagePayload = Pick +export type EditQueuedMessagePayload = Pick export interface WebviewMessage { type: @@ -609,6 +609,7 @@ export interface WebviewMessage { askResponse?: ClineAskResponse apiConfiguration?: ProviderSettings images?: string[] + deliveryMode?: QueuedMessage["deliveryMode"] bool?: boolean value?: number stepIndex?: number diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index 7f5862be15..8fdc172718 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -932,6 +932,22 @@ export async function presentAssistantMessage(cline: Task) { // locked. cline.presentAssistantMessageLocked = false + const completedToolBoundary = + (block.type === "tool_use" || block.type === "mcp_tool_use") && + !block.partial && + !cline.didRejectTool && + !cline.didAlreadyUseTool + + if (completedToolBoundary) { + const didInterruptForSteer = await cline.maybeInterruptForPendingSteerAtToolBoundary( + cline.currentStreamingContentIndex, + ) + + if (didInterruptForSteer) { + return + } + } + // NOTE: When tool is rejected, iterator stream is interrupted and it waits // for `userMessageContentReady` to be true. Future calls to present will // skip execution since `didRejectTool` and iterate until `contentIndex` is diff --git a/src/core/message-queue/MessageQueueService.ts b/src/core/message-queue/MessageQueueService.ts index fe38bf0194..97055a7f78 100644 --- a/src/core/message-queue/MessageQueueService.ts +++ b/src/core/message-queue/MessageQueueService.ts @@ -2,7 +2,7 @@ import { EventEmitter } from "events" import { v4 as uuidv4 } from "uuid" -import { QueuedMessage } from "@roo-code/types" +import { QueuedMessage, QueuedMessageDeliveryMode } from "@roo-code/types" export interface MessageQueueState { messages: QueuedMessage[] @@ -15,84 +15,222 @@ export interface QueueEvents { } export class MessageQueueService extends EventEmitter { - private _messages: QueuedMessage[] + private readonly lanes: Record constructor() { super() - this._messages = [] + this.lanes = { + queue: [], + steer: [], + } } - private findMessage(id: string) { - const index = this._messages.findIndex((msg) => msg.id === id) + private normalizeMessage( + message: Pick & Partial>, + ): QueuedMessage { + const createdAt = message.createdAt ?? message.timestamp ?? Date.now() + const updatedAt = message.updatedAt ?? message.timestamp ?? createdAt + + return { + timestamp: message.timestamp ?? updatedAt, + createdAt, + updatedAt, + id: message.id, + text: message.text, + images: message.images ? [...message.images] : undefined, + deliveryMode: message.deliveryMode ?? "queue", + } + } + + private emitStateChanged(): void { + this.emit("stateChanged", this.messages) + } - if (index === -1) { - return { index, message: undefined } + private getLane(deliveryMode: QueuedMessageDeliveryMode): QueuedMessage[] { + return this.lanes[deliveryMode] + } + + private findMessage(id: string) { + for (const deliveryMode of ["steer", "queue"] as const) { + const lane = this.getLane(deliveryMode) + const index = lane.findIndex((message) => message.id === id) + + if (index !== -1) { + return { + deliveryMode, + index, + message: lane[index], + } + } } - return { index, message: this._messages[index] } + return { + deliveryMode: undefined, + index: -1, + message: undefined, + } } - public addMessage(text: string, images?: string[]): QueuedMessage | undefined { + public addMessage( + text: string, + images?: string[], + deliveryMode: QueuedMessageDeliveryMode = "queue", + ): QueuedMessage | undefined { if (!text && !images?.length) { return undefined } + const now = Date.now() const message: QueuedMessage = { - timestamp: Date.now(), + timestamp: now, + createdAt: now, + updatedAt: now, id: uuidv4(), text, images, + deliveryMode, } - this._messages.push(message) - this.emit("stateChanged", this._messages) + this.getLane(deliveryMode).push(message) + this.emitStateChanged() return message } public removeMessage(id: string): boolean { - const { index, message } = this.findMessage(id) + const { deliveryMode, index, message } = this.findMessage(id) - if (!message) { + if (!message || !deliveryMode) { return false } - this._messages.splice(index, 1) - this.emit("stateChanged", this._messages) + this.getLane(deliveryMode).splice(index, 1) + this.emitStateChanged() return true } - public updateMessage(id: string, text: string, images?: string[]): boolean { + public updateMessage( + id: string, + text: string, + images?: string[], + deliveryMode?: QueuedMessageDeliveryMode, + ): boolean { + const now = Date.now() const { message } = this.findMessage(id) if (!message) { return false } - message.timestamp = Date.now() + const nextDeliveryMode = deliveryMode ?? message.deliveryMode + + if (nextDeliveryMode !== message.deliveryMode) { + return this.setDeliveryMode(id, nextDeliveryMode, { text, images, updatedAt: now }) + } + + message.timestamp = now + message.updatedAt = now message.text = text message.images = images - this.emit("stateChanged", this._messages) + this.emitStateChanged() return true } public dequeueMessage(): QueuedMessage | undefined { - const message = this._messages.shift() - this.emit("stateChanged", this._messages) + return this.dequeueNextMessage(["steer", "queue"]) + } + + public dequeueMessageByMode(deliveryMode: QueuedMessageDeliveryMode): QueuedMessage | undefined { + const message = this.getLane(deliveryMode).shift() + + if (message) { + this.emitStateChanged() + } + return message } + public dequeueNextMessage(priority: QueuedMessageDeliveryMode[] = ["steer", "queue"]): QueuedMessage | undefined { + for (const deliveryMode of priority) { + const message = this.dequeueMessageByMode(deliveryMode) + + if (message) { + return message + } + } + + return undefined + } + + public setDeliveryMode( + id: string, + deliveryMode: QueuedMessageDeliveryMode, + overrides?: { text?: string; images?: string[]; updatedAt?: number }, + ): boolean { + const { deliveryMode: currentDeliveryMode, index, message } = this.findMessage(id) + + if (!message || !currentDeliveryMode) { + return false + } + + const now = overrides?.updatedAt ?? Date.now() + const nextMessage: QueuedMessage = { + ...message, + timestamp: now, + updatedAt: now, + text: overrides?.text ?? message.text, + images: overrides?.images ?? message.images, + deliveryMode, + } + + if (currentDeliveryMode === deliveryMode) { + this.getLane(currentDeliveryMode)[index] = nextMessage + this.emitStateChanged() + return true + } + + this.getLane(currentDeliveryMode).splice(index, 1) + this.getLane(deliveryMode).push(nextMessage) + this.emitStateChanged() + return true + } + + public restoreMessages(messages: QueuedMessage[]): void { + this.lanes.queue = [] + this.lanes.steer = [] + + for (const message of messages) { + const normalized = this.normalizeMessage(message) + this.getLane(normalized.deliveryMode).push(normalized) + } + + this.emitStateChanged() + } + public get messages(): QueuedMessage[] { - return this._messages + return [...this.lanes.steer, ...this.lanes.queue] + } + + public getMessagesByMode(deliveryMode: QueuedMessageDeliveryMode): QueuedMessage[] { + return [...this.getLane(deliveryMode)] + } + + public hasMessages(deliveryMode?: QueuedMessageDeliveryMode): boolean { + if (deliveryMode) { + return this.getLane(deliveryMode).length > 0 + } + + return this.messages.length > 0 } - public isEmpty(): boolean { - return this._messages.length === 0 + public isEmpty(deliveryMode?: QueuedMessageDeliveryMode): boolean { + return !this.hasMessages(deliveryMode) } public dispose(): void { - this._messages = [] + this.lanes.queue = [] + this.lanes.steer = [] this.removeAllListeners() } } diff --git a/src/core/message-queue/__tests__/MessageQueueService.spec.ts b/src/core/message-queue/__tests__/MessageQueueService.spec.ts new file mode 100644 index 0000000000..8c7c57f498 --- /dev/null +++ b/src/core/message-queue/__tests__/MessageQueueService.spec.ts @@ -0,0 +1,37 @@ +import { MessageQueueService } from "../MessageQueueService" + +describe("MessageQueueService", () => { + it("keeps queue and steer lanes separate while exposing steer-first ordering", () => { + const service = new MessageQueueService() + + const queued = service.addMessage("queued", undefined, "queue") + const steer = service.addMessage("steer", undefined, "steer") + + expect(service.messages.map((message) => message.text)).toEqual(["steer", "queued"]) + expect(service.getMessagesByMode("queue")).toEqual([queued]) + expect(service.getMessagesByMode("steer")).toEqual([steer]) + + expect(service.dequeueMessageByMode("queue")?.text).toBe("queued") + expect(service.dequeueMessageByMode("steer")?.text).toBe("steer") + expect(service.isEmpty()).toBe(true) + }) + + it("can move messages between lanes without losing timestamps or images", () => { + const service = new MessageQueueService() + const message = service.addMessage("queued", ["image.png"], "queue") + + expect(message).toBeDefined() + expect(service.updateMessage(message!.id, "steer now", ["image.png"], "steer")).toBe(true) + + expect(service.getMessagesByMode("queue")).toEqual([]) + expect(service.getMessagesByMode("steer")).toEqual([ + expect.objectContaining({ + id: message!.id, + text: "steer now", + images: ["image.png"], + deliveryMode: "steer", + createdAt: message!.createdAt, + }), + ]) + }) +}) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index e19a1ff00d..9a44456a8e 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -335,6 +335,9 @@ export class Task extends EventEmitter implements TaskLike { // Message Queue Service public readonly messageQueueService: MessageQueueService private messageQueueStateChangedHandler: (() => void) | undefined + private deferQueuedMessageDrainUntilResume = false + private didSteerCurrentTurn = false + private interruptedAssistantText?: string // Streaming isWaitingForFirstChunk = false @@ -1429,11 +1432,12 @@ export class Task extends EventEmitter implements TaskLike { // The state is mutable if the message is complete and the task will // block (via the `pWaitFor`). const isBlocking = !(this.askResponse !== undefined || this.lastMessageTs !== askTs) - const isMessageQueued = !this.messageQueueService.isEmpty() - // Keep queued user messages intact during command_output asks. Those asks - // are terminal flow-control, not conversational turns. - const shouldDrainQueuedMessageForAsk = type !== "command_output" - const isStatusMutable = !partial && isBlocking && !isMessageQueued && approval.decision === "ask" + const hasDeferredMessage = !this.messageQueueService.isEmpty() + const shouldPauseQueuedDrainForAsk = this.deferQueuedMessageDrainUntilResume && isResumableAsk(type) + const shouldAutoDispatchDeferredMessageForAsk = + this.canAutoDispatchDeferredMessageForAsk(type) && !shouldPauseQueuedDrainForAsk + const hasAutoDispatchCandidate = hasDeferredMessage && shouldAutoDispatchDeferredMessageForAsk + const isStatusMutable = !partial && isBlocking && !hasAutoDispatchCandidate && approval.decision === "ask" if (isStatusMutable) { const statusMutationTimeout = 2_000 @@ -1473,44 +1477,25 @@ export class Task extends EventEmitter implements TaskLike { }, statusMutationTimeout), ) } - } else if (isMessageQueued && shouldDrainQueuedMessageForAsk) { - const message = this.messageQueueService.dequeueMessage() - - if (message) { - // Check if this is a tool approval ask that needs to be handled. - if (type === "tool" || type === "command" || type === "use_mcp_server") { - // For tool approvals, we need to approve first, then send - // the message if there's text/images. - this.handleWebviewAskResponse("yesButtonClicked", message.text, message.images) - } else { - // For other ask types (like followup or command_output), fulfill the ask - // directly. - this.handleWebviewAskResponse("messageResponse", message.text, message.images) - } - } + } else if (hasAutoDispatchCandidate) { + this.consumeDeferredMessageForAsk(type) } // Wait for askResponse to be set await pWaitFor( () => { + if (this.abort) { + return true + } if (this.askResponse !== undefined || this.lastMessageTs !== askTs) { return true } - // If a queued message arrives while we're blocked on an ask (e.g. a follow-up - // suggestion click that was incorrectly queued due to UI state), consume it - // immediately so the task doesn't hang. - if (shouldDrainQueuedMessageForAsk && !this.messageQueueService.isEmpty()) { - const message = this.messageQueueService.dequeueMessage() - if (message) { - // If this is a tool approval ask, we need to approve first (yesButtonClicked) - // and include any queued text/images. - if (type === "tool" || type === "command" || type === "use_mcp_server") { - this.handleWebviewAskResponse("yesButtonClicked", message.text, message.images) - } else { - this.handleWebviewAskResponse("messageResponse", message.text, message.images) - } - } + // If a deferred message arrives while we're blocked on a handoff ask (for example + // a follow-up suggestion click that was queued while the agent still owned the turn), + // consume it immediately so the task doesn't hang. + if (shouldAutoDispatchDeferredMessageForAsk && !this.messageQueueService.isEmpty()) { + this.consumeDeferredMessageForAsk(type) } return false @@ -1518,6 +1503,10 @@ export class Task extends EventEmitter implements TaskLike { { interval: 100 }, ) + if (this.abort) { + throw new Error(`[RooCode#ask] task ${this.taskId}.${this.instanceId} aborted`) + } + if (this.lastMessageTs !== askTs) { // Could happen if we send multiple asks in a row i.e. with // command_output. It's important that when we know an ask could @@ -1605,6 +1594,10 @@ export class Task extends EventEmitter implements TaskLike { } } + public setDeferQueuedMessageDrainUntilResume(value: boolean): void { + this.deferQueuedMessageDrainUntilResume = value + } + public approveAsk({ text, images }: { text?: string; images?: string[] } = {}) { this.handleWebviewAskResponse("yesButtonClicked", text, images) } @@ -1675,6 +1668,152 @@ export class Task extends EventEmitter implements TaskLike { } } + private canAutoDispatchDeferredMessageForAsk(type: ClineAsk): boolean { + return type === "followup" || isIdleAsk(type) + } + + private dequeueNextDeferredMessageForHandoff(): QueuedMessage | undefined { + return this.messageQueueService.dequeueNextMessage(["steer", "queue"]) + } + + private createUserMessageBlocks( + text?: string, + images?: string[], + ): Array { + const blocks: Array = [] + + if (text) { + blocks.push({ + type: "text", + text: `\n${text}\n`, + }) + } + + if (images?.length) { + blocks.push(...formatResponse.imageBlocks(images)) + } + + return blocks + } + + private appendDeferredMessageToUserContent(message: Pick): void { + this.userMessageContent.push(...this.createUserMessageBlocks(message.text, message.images)) + } + + private consumeDeferredMessageForAsk(type: ClineAsk): boolean { + if (!this.canAutoDispatchDeferredMessageForAsk(type)) { + return false + } + + const message = this.dequeueNextDeferredMessageForHandoff() + + if (!message) { + return false + } + + this.handleWebviewAskResponse("messageResponse", message.text, message.images) + return true + } + + private getAssistantTextUpToContentIndex(contentIndex: number): string { + let interruptedAssistantText = "" + + for (let index = 0; index <= Math.min(contentIndex, this.assistantMessageContent.length - 1); index++) { + const block = this.assistantMessageContent[index] + + if (block?.type === "text" && typeof block.content === "string") { + interruptedAssistantText = block.content + } + } + + return interruptedAssistantText + } + + private appendSyntheticToolResultsForSkippedBlocks(startingAfterIndex: number): void { + for (const block of this.assistantMessageContent.slice(startingAfterIndex + 1)) { + if ((block.type === "tool_use" || block.type === "mcp_tool_use") && !block.partial && "id" in block) { + const toolCallId = block.id + + if (toolCallId) { + this.pushToolResultToUserContent({ + type: "tool_result", + tool_use_id: sanitizeToolUseId(toolCallId), + content: "Task was interrupted before this tool call could be completed.", + }) + } + } + } + } + + private async appendUserFeedbackToLatestUserMessage(text?: string, images?: string[]): Promise { + const feedbackBlocks = this.createUserMessageBlocks(text, images) + + if (feedbackBlocks.length === 0) { + return + } + + const lastUserMessageIndex = findLastIndex( + this.apiConversationHistory, + (message) => !message.isSummary && message.role === "user", + ) + + if (lastUserMessageIndex === -1) { + await this.addToApiConversationHistory({ role: "user", content: feedbackBlocks }) + return + } + + const lastUserMessage = this.apiConversationHistory[lastUserMessageIndex] + const currentContent: Anthropic.Messages.ContentBlockParam[] = Array.isArray(lastUserMessage.content) + ? [...lastUserMessage.content] + : [{ type: "text", text: lastUserMessage.content }] + + const environmentDetailsIndex = currentContent.findIndex( + (block) => + block.type === "text" && + block.text.trim().startsWith("") && + block.text.trim().endsWith(""), + ) + + if (environmentDetailsIndex === -1) { + currentContent.push(...feedbackBlocks) + } else { + currentContent.splice(environmentDetailsIndex, 0, ...feedbackBlocks) + } + + lastUserMessage.content = currentContent + await this.saveApiConversationHistory() + } + + public async maybeInterruptForPendingSteerAtToolBoundary(completedContentIndex: number): Promise { + const message = this.messageQueueService.dequeueMessageByMode("steer") + + if (!message) { + return false + } + + await this.say("user_feedback", message.text, message.images) + this.interruptedAssistantText = this.getAssistantTextUpToContentIndex(completedContentIndex) + this.appendSyntheticToolResultsForSkippedBlocks(completedContentIndex) + this.appendDeferredMessageToUserContent(message) + this.currentStreamingContentIndex = this.assistantMessageContent.length + this.userMessageContentReady = true + this.didSteerCurrentTurn = true + + return true + } + + public async consumePendingSteerAtApiBoundary(): Promise { + const message = this.messageQueueService.dequeueMessageByMode("steer") + + if (!message) { + return false + } + + await this.say("user_feedback", message.text, message.images) + this.appendDeferredMessageToUserContent(message) + return true + } + async handleTerminalOperation(terminalOperation: "continue" | "abort") { if (terminalOperation === "continue") { this.terminalProcess?.continue() @@ -1794,9 +1933,6 @@ export class Task extends EventEmitter implements TaskLike { { isNonInteractive: true } /* options */, contextCondense, ) - - // Process any queued messages after condensing completes - this.processQueuedMessages() } async say( @@ -2113,14 +2249,32 @@ export class Task extends EventEmitter implements TaskLike { this.isInitialized = true const { response, text, images } = await this.ask(askType) // Calls `postStateToWebview`. + const shouldUseQueuedMessageOnResume = this.deferQueuedMessageDrainUntilResume + this.deferQueuedMessageDrainUntilResume = false + + let responseText: string | undefined = + response === "messageResponse" || response === "yesButtonClicked" ? text : undefined + let responseImages: string[] | undefined = + response === "messageResponse" || response === "yesButtonClicked" ? images : undefined + + if ( + shouldUseQueuedMessageOnResume && + response === "yesButtonClicked" && + !responseText && + (responseImages?.length ?? 0) === 0 + ) { + const queuedMessage = this.dequeueNextDeferredMessageForHandoff() + if (queuedMessage) { + responseText = queuedMessage.text || undefined + responseImages = queuedMessage.images + } + } - let responseText: string | undefined - let responseImages: string[] | undefined - - if (response === "messageResponse") { - await this.say("user_feedback", text, images) - responseText = text - responseImages = images + if ( + (response === "messageResponse" || response === "yesButtonClicked") && + (responseText || responseImages?.length) + ) { + await this.say("user_feedback", responseText, responseImages) } // Make sure that the api conversation history can be resumed by the API, @@ -2810,6 +2964,8 @@ export class Task extends EventEmitter implements TaskLike { this.userMessageContent = [] this.userMessageContentReady = false this.didRejectTool = false + this.didSteerCurrentTurn = false + this.interruptedAssistantText = undefined this.didAlreadyUseTool = false this.assistantMessageSavedToHistory = false // Reset tool failure flag for each new assistant turn - this ensures that tool failures @@ -3106,6 +3262,9 @@ export class Task extends EventEmitter implements TaskLike { // this.userMessageContentReady = true break } + if (this.didSteerCurrentTurn) { + break + } if (this.didAlreadyUseTool) { assistantMessage += @@ -3488,8 +3647,10 @@ export class Task extends EventEmitter implements TaskLike { // the assistant message is already in history. Otherwise, tool_result blocks would appear // BEFORE their corresponding tool_use blocks, causing API errors. + const assistantTextForHistory = this.interruptedAssistantText ?? assistantMessage + // Check if we have any content to process (text or tool uses) - const hasTextContent = assistantMessage.length > 0 + const hasTextContent = assistantTextForHistory.length > 0 const hasToolUses = this.assistantMessageContent.some( (block) => block.type === "tool_use" || block.type === "mcp_tool_use", @@ -3512,10 +3673,10 @@ export class Task extends EventEmitter implements TaskLike { const assistantContent: Array = [] // Add text content if present - if (assistantMessage) { + if (assistantTextForHistory) { assistantContent.push({ type: "text" as const, - text: assistantMessage, + text: assistantTextForHistory, }) } @@ -3692,6 +3853,8 @@ export class Task extends EventEmitter implements TaskLike { this.consecutiveNoToolUseCount = 0 } + await this.consumePendingSteerAtApiBoundary() + // Push to stack if there's content OR if we're paused waiting for a subtask. // When paused, we push an empty item so the loop continues to the pause check. if (this.userMessageContent.length > 0 || this.isPaused) { @@ -4416,12 +4579,15 @@ export class Task extends EventEmitter implements TaskLike { return } else { - const { response } = await this.ask( + const { response, text, images } = await this.ask( "api_req_failed", error.message ?? JSON.stringify(serializeError(error), null, 2), ) - if (response !== "yesButtonClicked") { + if (response === "messageResponse") { + await this.say("user_feedback", text ?? "", images) + await this.appendUserFeedbackToLatestUserMessage(text, images) + } else if (response !== "yesButtonClicked") { // This will never happen since if noButtonClicked, we will // clear current task, aborting this instance. throw new Error("API request failed") @@ -4776,26 +4942,11 @@ export class Task extends EventEmitter implements TaskLike { } /** - * Process any queued messages by dequeuing and submitting them. - * This ensures that queued user messages are sent when appropriate, - * preventing them from getting stuck in the queue. - * - * @param context - Context string for logging (e.g., the calling tool name) + * Legacy no-op kept for compatibility with older call sites and tests. + * Deferred messages are now dispatched only through explicit queue/steer + * handoff and safe-boundary helpers. */ public processQueuedMessages(): void { - try { - if (!this.messageQueueService.isEmpty()) { - const queued = this.messageQueueService.dequeueMessage() - if (queued) { - setTimeout(() => { - this.submitUserMessage(queued.text, queued.images).catch((err) => - console.error(`[Task] Failed to submit queued message:`, err), - ) - }, 0) - } - } - } catch (e) { - console.error(`[Task] Queue processing error:`, e) - } + return } } diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 6a65c858f9..6bdf6e7f04 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -1755,7 +1755,7 @@ describe("Cline", () => { }) }) -describe("Queued message processing after condense", () => { +describe("Deferred message dispatch boundaries", () => { function createProvider(): any { const storageUri = { fsPath: path.join(os.tmpdir(), "test-storage") } const ctx = { @@ -1802,7 +1802,7 @@ describe("Queued message processing after condense", () => { apiKey: "test-api-key", } as any - it("processes queued message after condense completes", async () => { + it("keeps queued messages pending when processQueuedMessages is called", async () => { const provider = createProvider() const task = new Task({ provider, @@ -1810,70 +1810,104 @@ describe("Queued message processing after condense", () => { task: "initial task", startTask: false, }) - - // Make condense fast + deterministic - vi.spyOn(task as any, "getSystemPrompt").mockResolvedValue("system") const submitSpy = vi.spyOn(task, "submitUserMessage").mockResolvedValue(undefined) + task.messageQueueService.addMessage("queued text", ["img1.png"], "queue") - // Queue a message during condensing - task.messageQueueService.addMessage("queued text", ["img1.png"]) - - // Use fake timers to capture setTimeout(0) in processQueuedMessages - vi.useFakeTimers() - await task.condenseContext() + task.processQueuedMessages() - // Flush the microtask that submits the queued message - vi.runAllTimers() - vi.useRealTimers() - - expect(submitSpy).toHaveBeenCalledWith("queued text", ["img1.png"]) - expect(task.messageQueueService.isEmpty()).toBe(true) + expect(submitSpy).not.toHaveBeenCalled() + expect(task.messageQueueService.getMessagesByMode("queue")).toEqual([ + expect.objectContaining({ + text: "queued text", + images: ["img1.png"], + deliveryMode: "queue", + }), + ]) }) - it("does not cross-drain queues between separate tasks", async () => { - const providerA = createProvider() - const providerB = createProvider() - - const taskA = new Task({ - provider: providerA, - apiConfiguration: apiConfig, - task: "task A", - startTask: false, - }) - const taskB = new Task({ - provider: providerB, + it("consumes only steer messages at API boundaries and preserves queued follow-ups", async () => { + const provider = createProvider() + const task = new Task({ + provider, apiConfiguration: apiConfig, - task: "task B", + task: "initial task", startTask: false, }) - vi.spyOn(taskA as any, "getSystemPrompt").mockResolvedValue("system") - vi.spyOn(taskB as any, "getSystemPrompt").mockResolvedValue("system") - - const spyA = vi.spyOn(taskA, "submitUserMessage").mockResolvedValue(undefined) - const spyB = vi.spyOn(taskB, "submitUserMessage").mockResolvedValue(undefined) + const saySpy = vi.spyOn(task, "say").mockResolvedValue(undefined as any) - taskA.messageQueueService.addMessage("A message") - taskB.messageQueueService.addMessage("B message") + task.messageQueueService.addMessage("queued text", undefined, "queue") + task.messageQueueService.addMessage("steer text", undefined, "steer") - // Condense in task A should only drain A's queue - vi.useFakeTimers() - await taskA.condenseContext() - vi.runAllTimers() - vi.useRealTimers() + const consumed = await task.consumePendingSteerAtApiBoundary() - expect(spyA).toHaveBeenCalledWith("A message", undefined) - expect(spyB).not.toHaveBeenCalled() - expect(taskB.messageQueueService.isEmpty()).toBe(false) + expect(consumed).toBe(true) + expect(saySpy).toHaveBeenCalledWith("user_feedback", "steer text", undefined) + expect(task.messageQueueService.getMessagesByMode("steer")).toEqual([]) + expect(task.messageQueueService.getMessagesByMode("queue")).toEqual([ + expect.objectContaining({ + text: "queued text", + deliveryMode: "queue", + }), + ]) + expect((task as any).userMessageContent).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + type: "text", + text: "\nsteer text\n", + }), + ]), + ) + }) - // Now condense in task B should drain B's queue - vi.useFakeTimers() - await taskB.condenseContext() - vi.runAllTimers() - vi.useRealTimers() + it("interrupts after a tool boundary for steer messages and injects synthetic tool results for skipped tools", async () => { + const provider = createProvider() + const task = new Task({ + provider, + apiConfiguration: apiConfig, + task: "initial task", + startTask: false, + }) - expect(spyB).toHaveBeenCalledWith("B message", undefined) - expect(taskB.messageQueueService.isEmpty()).toBe(true) + const saySpy = vi.spyOn(task, "say").mockResolvedValue(undefined as any) + + ;(task as any).assistantMessageContent = [ + { type: "text", content: "first plan" }, + { type: "tool_use", id: "tool-1", partial: false, name: "read_file" }, + { type: "tool_use", id: "tool-2", partial: false, name: "edit_file" }, + ] + + task.messageQueueService.addMessage("queued follow-up", undefined, "queue") + task.messageQueueService.addMessage("steer follow-up", undefined, "steer") + + const interrupted = await task.maybeInterruptForPendingSteerAtToolBoundary(1) + + expect(interrupted).toBe(true) + expect(saySpy).toHaveBeenCalledWith("user_feedback", "steer follow-up", undefined) + expect((task as any).interruptedAssistantText).toBe("first plan") + expect((task as any).currentStreamingContentIndex).toBe((task as any).assistantMessageContent.length) + expect((task as any).userMessageContentReady).toBe(true) + expect((task as any).didSteerCurrentTurn).toBe(true) + expect((task as any).userMessageContent).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + type: "tool_result", + tool_use_id: "tool-2", + content: "Task was interrupted before this tool call could be completed.", + }), + expect.objectContaining({ + type: "text", + text: "\nsteer follow-up\n", + }), + ]), + ) + expect(task.messageQueueService.getMessagesByMode("steer")).toEqual([]) + expect(task.messageQueueService.getMessagesByMode("queue")).toEqual([ + expect.objectContaining({ + text: "queued follow-up", + deliveryMode: "queue", + }), + ]) }) }) diff --git a/src/core/task/__tests__/ask-queued-message-drain.spec.ts b/src/core/task/__tests__/ask-queued-message-drain.spec.ts index 06f577881e..3141f1e698 100644 --- a/src/core/task/__tests__/ask-queued-message-drain.spec.ts +++ b/src/core/task/__tests__/ask-queued-message-drain.spec.ts @@ -69,4 +69,40 @@ describe("Task.ask queued message drain", () => { expect((task as any).messageQueueService.isEmpty()).toBe(false) expect((task as any).messageQueueService.messages[0]?.text).toBe("1+1=?") }) + + it("does not consume queued messages for resumable asks when draining is deferred", async () => { + const task = Object.create(Task.prototype) as Task + ;(task as any).abort = false + ;(task as any).clineMessages = [] + ;(task as any).askResponse = undefined + ;(task as any).askResponseText = undefined + ;(task as any).askResponseImages = undefined + ;(task as any).lastMessageTs = undefined + + const { MessageQueueService } = await import("../../message-queue/MessageQueueService") + ;(task as any).messageQueueService = new MessageQueueService() + ;(task as any).addToClineMessages = vi.fn(async () => {}) + ;(task as any).saveClineMessages = vi.fn(async () => {}) + ;(task as any).updateClineMessage = vi.fn(async () => {}) + ;(task as any).cancelAutoApprovalTimeout = vi.fn(() => {}) + ;(task as any).checkpointSave = vi.fn(async () => {}) + ;(task as any).emit = vi.fn() + ;(task as any).providerRef = { deref: () => undefined } + + task.setDeferQueuedMessageDrainUntilResume(true) + + const askPromise = task.ask("resume_task", "resume?", false) + ;(task as any).messageQueueService.addMessage("follow up after cancel") + + setTimeout(() => { + task.approveAsk() + }, 0) + + const result = await askPromise + + expect(result.response).toBe("yesButtonClicked") + expect(result.text).toBeUndefined() + expect((task as any).messageQueueService.isEmpty()).toBe(false) + expect((task as any).messageQueueService.messages[0]?.text).toBe("follow up after cancel") + }) }) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 1985a8b665..263043b54b 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -1270,12 +1270,16 @@ export class ClineProvider const wasVisible = this.visibleTaskId === oldTask?.taskId // Abort the old task to stop running processes and mark as abandoned - try { - await oldTask.abortTask(true) - } catch (e) { - this.log( - `[createTaskWithHistoryItem] abortTask() failed for old task ${oldTask.taskId}.${oldTask.instanceId}: ${e.message}`, - ) + if (oldTask && !oldTask.abandoned) { + try { + await oldTask.abortTask(true) + } catch (error) { + this.log( + `[createTaskWithHistoryItem] abortTask() failed for old task ${oldTask.taskId}.${oldTask.instanceId}: ${ + error instanceof Error ? error.message : String(error) + }`, + ) + } } // Remove event listeners from the old task @@ -3188,6 +3192,10 @@ export class ClineProvider console.log(`[cancelTask] cancelling task ${task.taskId}.${task.instanceId}`) const wasVisible = this.isTaskVisible(task.taskId) + const preservedQueuedMessages = task.messageQueueService.messages.map((message) => ({ + ...message, + images: message.images ? [...message.images] : undefined, + })) let historyItem: HistoryItem | undefined try { @@ -3265,10 +3273,15 @@ export class ClineProvider } // Clears task again, so we need to abortTask manually above. - await this.createTaskWithHistoryItem( + const replacementTask = await this.createTaskWithHistoryItem( { ...historyItem, rootTask, parentTask }, { replaceExistingTask: true, focus: wasVisible }, ) + + if (replacementTask && preservedQueuedMessages.length > 0) { + replacementTask.messageQueueService.restoreMessages(preservedQueuedMessages) + replacementTask.setDeferQueuedMessageDrainUntilResume(true) + } } // Clear the current task without treating it as a subtask. diff --git a/src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts b/src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts index 434c751779..7e5542c7a5 100644 --- a/src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts @@ -193,6 +193,8 @@ describe("ClineProvider flicker-free cancel", () => { emit: vi.fn(), on: vi.fn(), off: vi.fn(), + messageQueueService: { restoreMessages: vi.fn() }, + setDeferQueuedMessageDrainUntilResume: vi.fn(), } // Mock Task constructor @@ -341,4 +343,24 @@ describe("ClineProvider flicker-free cancel", () => { expect(mockTask1.abortTask).toHaveBeenCalledWith(true) expect(cleanup).toHaveBeenCalled() }) + + it("restores queued messages on the replacement task after cancel", async () => { + const queuedMessage = { + id: "queued-1", + text: "follow-up while streaming", + images: ["image-1.png"], + timestamp: 123, + createdAt: 123, + updatedAt: 123, + deliveryMode: "steer" as const, + } + + mockTask1.messageQueueService = { messages: [queuedMessage] } + ;(provider as any).clineStack = [mockTask1] + + await provider.cancelTask("task-1") + + expect(mockTask2.messageQueueService.restoreMessages).toHaveBeenCalledWith([queuedMessage]) + expect(mockTask2.setDeferQueuedMessageDrainUntilResume).toHaveBeenCalledWith(true) + }) }) diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index e9ce823e5f..c1e75a455e 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -3244,13 +3244,21 @@ export const webviewMessageHandler = async ( */ case "queueMessage": { - const resolved = await resolveIncomingImages({ text: message.text, images: message.images }) const targetTask = provider.resolveMessageTask(message.taskId) if (!targetTask) { + await restoreDroppedInput(message.text, message.images) + break + } + + const expectedTaskId = targetTask.taskId + const resolved = await resolveIncomingImages({ text: message.text, images: message.images }) + const stillActive = provider.resolveMessageTask(expectedTaskId) + if (!stillActive || stillActive.taskId !== expectedTaskId) { await restoreDroppedInput(resolved.text, resolved.images) break } - targetTask.messageQueueService.addMessage(resolved.text, resolved.images) + + stillActive.messageQueueService.addMessage(resolved.text, resolved.images, message.deliveryMode ?? "queue") break } case "removeQueuedMessage": { @@ -3259,8 +3267,10 @@ export const webviewMessageHandler = async ( } case "editQueuedMessage": { if (message.payload) { - const { id, text, images } = message.payload as EditQueuedMessagePayload - provider.resolveMessageTask(message.taskId)?.messageQueueService.updateMessage(id, text, images) + const { id, text, images, deliveryMode } = message.payload as EditQueuedMessagePayload + provider + .resolveMessageTask(message.taskId) + ?.messageQueueService.updateMessage(id, text, images, deliveryMode) } break diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index 039968aeef..9bdf2a0302 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -703,7 +703,13 @@ const ChatViewComponent: React.ForwardRefRenderFunction { - if (visibleMessageQueue[index]) { - vscode.postMessage({ - type: "removeQueuedMessage", - text: visibleMessageQueue[index].id, - taskId: currentTaskId, - }) - } + onRemove={(messageId) => { + vscode.postMessage({ + type: "removeQueuedMessage", + text: messageId, + taskId: currentTaskId, + }) }} - onUpdate={(index, newText) => { - if (visibleMessageQueue[index]) { - vscode.postMessage({ - type: "editQueuedMessage", - taskId: currentTaskId, - payload: { - id: visibleMessageQueue[index].id, - text: newText, - images: visibleMessageQueue[index].images, - }, - }) - } + onUpdate={(message, updates) => { + vscode.postMessage({ + type: "editQueuedMessage", + taskId: currentTaskId, + payload: { + id: message.id, + text: updates.text ?? message.text, + images: message.images, + deliveryMode: updates.deliveryMode ?? message.deliveryMode, + }, + }) }} /> {showRetiredProviderWarning && ( diff --git a/webview-ui/src/components/chat/QueuedMessages.tsx b/webview-ui/src/components/chat/QueuedMessages.tsx index 5bfb6fdb48..068da6d66a 100644 --- a/webview-ui/src/components/chat/QueuedMessages.tsx +++ b/webview-ui/src/components/chat/QueuedMessages.tsx @@ -1,4 +1,4 @@ -import { useState } from "react" +import { useLayoutEffect, useRef, useState } from "react" import { useTranslation } from "react-i18next" import { QueuedMessage } from "@roo-code/types" @@ -11,8 +11,54 @@ import { Mention } from "./Mention" interface QueuedMessagesProps { queue: QueuedMessage[] - onRemove: (index: number) => void - onUpdate: (index: number, newText: string) => void + onRemove: (messageId: string) => void + onUpdate: (message: QueuedMessage, updates: { text?: string; deliveryMode?: QueuedMessage["deliveryMode"] }) => void +} + +interface QueuedMessageEditorProps { + value: string + onChange: (value: string) => void + onSave: () => void + onCancel: () => void + placeholder: string + rows: number +} + +const QueuedMessageEditor = ({ value, onChange, onSave, onCancel, placeholder, rows }: QueuedMessageEditorProps) => { + const textareaRef = useRef(null) + const didPlaceInitialCaretRef = useRef(false) + + useLayoutEffect(() => { + if (didPlaceInitialCaretRef.current || !textareaRef.current) { + return + } + + didPlaceInitialCaretRef.current = true + const textarea = textareaRef.current + textarea.focus() + textarea.setSelectionRange(textarea.value.length, textarea.value.length) + }, []) + + return ( +