From 638ca0e09b914dbcc01202f25ca1845185c14708 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Sun, 2 Nov 2025 18:56:23 -0500 Subject: [PATCH 1/5] fix: prevent UI flicker and enable resumption after task cancellation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Problem: - Clicking Terminate during streaming caused the entire chat view to blink - Task would close, user pushed to home, then back into the task - Cancel → Resume flow would grey the Resume button but not restart the loop - Input would become disabled and messages couldn't be sent Root causes: 1. Hard abort (abandoned=true) was disposing the task instance immediately 2. Provider was rehydrating a new task instance from disk after abort 3. Abort flags stayed set (abort=true) preventing loop resumption 4. No deterministic spinner stopping on cancellation 5. Streaming state wasn't reset on resume Solution: - Soft-interrupt: Cancel now uses abortTask(false) to keep instance alive - No rehydration: Provider skips task recreation for user_cancelled aborts - Resume pipeline: Added presentResumableAsk() for in-place resume flow - Abort state reset: Clear abort flags and streaming state before resuming - Spinner determinism: Inject cancelReason into last api_req_started - Transactional UI: Pause state updates during checkpoint operations Changes: - Task.presentResumableAsk(): New method for soft-interrupt resume - Task.resumeTaskFromHistory(): Reset abort/streaming state after user confirms - Task.abortTask(): Distinguish soft (keep alive) vs hard abort (dispose) - ClineProvider.cancelTask(): Soft abort + schedule presentResumableAsk() - ClineProvider.onTaskAborted: Skip rehydration for user_cancelled - ChatView: Handle cancelReason for deterministic streaming state Tests: - Task.presentResumableAsk.abort-reset.spec.ts: Verify abort flag reset - ClineProvider.cancelTask.present-ask.spec.ts: Verify soft-interrupt flow - Task.spec.ts: Updated abort semantics (soft vs hard) Result: - No UI flicker or navigation on Cancel - Resume button successfully restarts the agent loop - Input remains enabled throughout cancel/resume cycle - Spinner stops deterministically on cancellation - Task history stable, no duplicate entries --- src/core/task/Task.ts | 114 +++++++++++++- ...sk.presentResumableAsk.abort-reset.spec.ts | 146 ++++++++++++++++++ src/core/task/__tests__/Task.spec.ts | 10 +- src/core/webview/ClineProvider.ts | 98 ++++++++---- ...ineProvider.cancelTask.present-ask.spec.ts | 60 +++++++ src/core/webview/webviewMessageHandler.ts | 26 ++-- webview-ui/src/components/chat/ChatView.tsx | 10 +- 7 files changed, 408 insertions(+), 56 deletions(-) create mode 100644 src/core/task/__tests__/Task.presentResumableAsk.abort-reset.spec.ts create mode 100644 src/core/webview/__tests__/ClineProvider.cancelTask.present-ask.spec.ts diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 9c199a2a1bf..9c44bbb9561 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -737,7 +737,8 @@ export class Task extends EventEmitter implements TaskLike { // deallocated. (Although we set Cline = undefined in provider, that // simply removes the reference to this instance, but the instance is // still alive until this promise resolves or rejects.) - if (this.abort) { + // Exception: Allow resume asks even when aborted for soft-interrupt UX + if (this.abort && type !== "resume_task" && type !== "resume_completed_task") { throw new Error(`[RooCode#ask] task ${this.taskId}.${this.instanceId} aborted`) } @@ -1255,7 +1256,7 @@ export class Task extends EventEmitter implements TaskLike { ]) } - private async resumeTaskFromHistory() { + public async resumeTaskFromHistory() { if (this.enableBridge) { try { await BridgeOrchestrator.subscribeToTask(this) @@ -1347,6 +1348,30 @@ export class Task extends EventEmitter implements TaskLike { const { response, text, images } = await this.ask(askType) // Calls `postStateToWebview`. + // Reset abort flags AFTER user responds to resume ask. + // This is critical for the cancel → resume flow: when a task is soft-aborted + // (abandoned = false), we keep the instance alive but set abort = true. + // We only clear these flags after the user confirms they want to resume, + // preventing the old stream from continuing if abort was set. + this.abort = false + this.abandoned = false + this.abortReason = undefined + this.didFinishAbortingStream = false + this.isStreaming = false + + // Reset streaming-local fields to avoid stale state from previous stream + this.currentStreamingContentIndex = 0 + this.currentStreamingDidCheckpoint = false + this.assistantMessageContent = [] + this.didCompleteReadingStream = false + this.userMessageContent = [] + this.userMessageContentReady = false + this.didRejectTool = false + this.didAlreadyUseTool = false + this.presentAssistantMessageLocked = false + this.presentAssistantMessageHasPendingUpdates = false + this.assistantMessageParser.reset() + let responseText: string | undefined let responseImages: string[] | undefined @@ -1525,6 +1550,76 @@ export class Task extends EventEmitter implements TaskLike { await this.initiateTaskLoop(newUserContent) } + /** + * Present a resumable ask on an aborted task without rehydrating. + * Used by soft-interrupt (cancelTask) to show Resume/Terminate UI. + * Selects the appropriate ask type based on the last relevant message. + * If the user clicks Resume, resets abort flags and continues the task loop. + */ + public async presentResumableAsk(): Promise { + const lastClineMessage = this.clineMessages + .slice() + .reverse() + .find((m) => !(m.ask === "resume_task" || m.ask === "resume_completed_task")) + + let askType: ClineAsk + if (lastClineMessage?.ask === "completion_result") { + askType = "resume_completed_task" + } else { + askType = "resume_task" + } + + const { response, text, images } = await this.ask(askType) + + // If user clicked Resume (not Terminate), reset abort flags and continue + if (response === "yesButtonClicked" || response === "messageResponse") { + // Reset abort flags to allow the loop to continue + this.abort = false + this.abandoned = false + this.abortReason = undefined + this.didFinishAbortingStream = false + this.isStreaming = false + + // Reset streaming-local fields to avoid stale state from previous stream + this.currentStreamingContentIndex = 0 + this.currentStreamingDidCheckpoint = false + this.assistantMessageContent = [] + this.didCompleteReadingStream = false + this.userMessageContent = [] + this.userMessageContentReady = false + this.didRejectTool = false + this.didAlreadyUseTool = false + this.presentAssistantMessageLocked = false + this.presentAssistantMessageHasPendingUpdates = false + this.assistantMessageParser.reset() + + // Prepare content for resuming the task loop + let userContent: Anthropic.Messages.ContentBlockParam[] = [] + + if (response === "messageResponse" && text) { + // User provided additional instructions + await this.say("user_feedback", text, images) + userContent.push({ + type: "text", + text: `\n\nNew instructions for task continuation:\n\n${text}\n`, + }) + if (images && images.length > 0) { + userContent.push(...formatResponse.imageBlocks(images)) + } + } else { + // Simple resume with no new instructions + userContent.push({ + type: "text", + text: "[TASK RESUMPTION] Resuming task...", + }) + } + + // Continue the task loop + await this.initiateTaskLoop(userContent) + } + // If user clicked Terminate (noButtonClicked), do nothing - task stays aborted + } + public async abortTask(isAbandoned = false) { // Aborting task @@ -1536,12 +1631,17 @@ export class Task extends EventEmitter implements TaskLike { this.abort = true this.emit(RooCodeEventName.TaskAborted) - try { - this.dispose() // Call the centralized dispose method - } catch (error) { - console.error(`Error during task ${this.taskId}.${this.instanceId} disposal:`, error) - // Don't rethrow - we want abort to always succeed + // Only dispose if this is a hard abort (abandoned) + // For soft abort (user cancel), keep the instance alive so we can present a resumable ask + if (isAbandoned) { + try { + this.dispose() // Call the centralized dispose method + } catch (error) { + console.error(`Error during task ${this.taskId}.${this.instanceId} disposal:`, error) + // Don't rethrow - we want abort to always succeed + } } + // Save the countdown message in the automatic retry or other content. try { // Save the countdown message in the automatic retry or other content. diff --git a/src/core/task/__tests__/Task.presentResumableAsk.abort-reset.spec.ts b/src/core/task/__tests__/Task.presentResumableAsk.abort-reset.spec.ts new file mode 100644 index 00000000000..f37cfeccf80 --- /dev/null +++ b/src/core/task/__tests__/Task.presentResumableAsk.abort-reset.spec.ts @@ -0,0 +1,146 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest" +import { ProviderSettings } from "@roo-code/types" + +import { Task } from "../Task" +import { ClineProvider } from "../../webview/ClineProvider" + +// Mocks similar to Task.dispose.test.ts +vi.mock("../../webview/ClineProvider") +vi.mock("../../../integrations/terminal/TerminalRegistry", () => ({ + TerminalRegistry: { + releaseTerminalsForTask: vi.fn(), + }, +})) +vi.mock("../../ignore/RooIgnoreController") +vi.mock("../../protect/RooProtectedController") +vi.mock("../../context-tracking/FileContextTracker") +vi.mock("../../../services/browser/UrlContentFetcher") +vi.mock("../../../services/browser/BrowserSession") +vi.mock("../../../integrations/editor/DiffViewProvider") +vi.mock("../../tools/ToolRepetitionDetector") +vi.mock("../../../api", () => ({ + buildApiHandler: vi.fn(() => ({ + getModel: () => ({ info: {}, id: "test-model" }), + })), +})) +vi.mock("../AutoApprovalHandler") + +// Mock TelemetryService +vi.mock("@roo-code/telemetry", () => ({ + TelemetryService: { + instance: { + captureTaskCreated: vi.fn(), + captureTaskRestarted: vi.fn(), + captureConversationMessage: vi.fn(), + }, + }, +})) + +describe("Task.presentResumableAsk abort reset", () => { + let mockProvider: any + let mockApiConfiguration: ProviderSettings + let task: Task + + beforeEach(() => { + vi.clearAllMocks() + + mockProvider = { + context: { + globalStorageUri: { fsPath: "/test/path" }, + }, + getState: vi.fn().mockResolvedValue({ mode: "code" }), + postStateToWebview: vi.fn().mockResolvedValue(undefined), + postMessageToWebview: vi.fn().mockResolvedValue(undefined), + updateTaskHistory: vi.fn().mockResolvedValue(undefined), + log: vi.fn(), + } + + mockApiConfiguration = { + apiProvider: "anthropic", + apiKey: "test-key", + } as ProviderSettings + + task = new Task({ + provider: mockProvider as ClineProvider, + apiConfiguration: mockApiConfiguration, + startTask: false, + }) + }) + + afterEach(() => { + // Ensure we don't leave event listeners dangling + task.dispose() + }) + + it("resets abort flags and continues the loop on yesButtonClicked", async () => { + // Arrange aborted state + task.abort = true + task.abortReason = "user_cancelled" + task.didFinishAbortingStream = true + task.isStreaming = true + + // minimal message history + task.clineMessages = [{ ts: Date.now() - 1000, type: "say", say: "text", text: "prev" } as any] + + // Spy and stub ask + loop + const askSpy = vi.spyOn(task as any, "ask").mockResolvedValue({ response: "yesButtonClicked" }) + const loopSpy = vi.spyOn(task as any, "initiateTaskLoop").mockResolvedValue(undefined) + + // Act + await task.presentResumableAsk() + + // Assert ask was presented + expect(askSpy).toHaveBeenCalled() + + // Abort flags cleared + expect(task.abort).toBe(false) + expect(task.abandoned).toBe(false) + expect(task.abortReason).toBeUndefined() + expect(task.didFinishAbortingStream).toBe(false) + expect(task.isStreaming).toBe(false) + + // Streaming-local state cleared + expect(task.currentStreamingContentIndex).toBe(0) + expect(task.assistantMessageContent).toEqual([]) + expect(task.userMessageContentReady).toBe(false) + expect(task.didRejectTool).toBe(false) + expect(task.presentAssistantMessageLocked).toBe(false) + + // Loop resumed + expect(loopSpy).toHaveBeenCalledTimes(1) + }) + + it("includes user feedback when resuming with messageResponse", async () => { + task.abort = true + task.clineMessages = [{ ts: Date.now() - 1000, type: "say", say: "text", text: "prev" } as any] + + const askSpy = vi + .spyOn(task as any, "ask") + .mockResolvedValue({ response: "messageResponse", text: "Continue with this", images: undefined }) + const saySpy = vi.spyOn(task, "say").mockResolvedValue(undefined as any) + const loopSpy = vi.spyOn(task as any, "initiateTaskLoop").mockResolvedValue(undefined) + + await task.presentResumableAsk() + + expect(askSpy).toHaveBeenCalled() + expect(saySpy).toHaveBeenCalledWith("user_feedback", "Continue with this", undefined) + expect(loopSpy).toHaveBeenCalledTimes(1) + }) + + it("does nothing when user clicks Terminate (noButtonClicked)", async () => { + task.abort = true + task.abortReason = "user_cancelled" + task.clineMessages = [{ ts: Date.now() - 1000, type: "say", say: "text", text: "prev" } as any] + + vi.spyOn(task as any, "ask").mockResolvedValue({ response: "noButtonClicked" }) + const loopSpy = vi.spyOn(task as any, "initiateTaskLoop").mockResolvedValue(undefined) + + await task.presentResumableAsk() + + // Still aborted + expect(task.abort).toBe(true) + expect(task.abortReason).toBe("user_cancelled") + // No loop resume + expect(loopSpy).not.toHaveBeenCalled() + }) +}) diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index b4f2e041634..3e79f090100 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -1722,12 +1722,12 @@ describe("Cline", () => { // Mock the dispose method to track cleanup const disposeSpy = vi.spyOn(task, "dispose").mockImplementation(() => {}) - // Call abortTask + // Call abortTask (soft cancel - same path as UI Cancel button) await task.abortTask() - // Verify the same behavior as Cancel button + // Verify the same behavior as Cancel button: soft abort sets abort flag but does not dispose expect(task.abort).toBe(true) - expect(disposeSpy).toHaveBeenCalled() + expect(disposeSpy).not.toHaveBeenCalled() }) it("should work with TaskLike interface", async () => { @@ -1771,8 +1771,8 @@ describe("Cline", () => { // Spy on console.error to verify error is logged const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}) - // abortTask should not throw even if dispose fails - await expect(task.abortTask()).resolves.not.toThrow() + // abortTask should not throw even if dispose fails (hard abort triggers dispose) + await expect(task.abortTask(true)).resolves.not.toThrow() // Verify error was logged expect(consoleErrorSpy).toHaveBeenCalledWith(expect.stringContaining("Error during task"), mockError) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 2a5fc24adab..8878ba35af7 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -143,6 +143,10 @@ export class ClineProvider private pendingOperations: Map = new Map() private static readonly PENDING_OPERATION_TIMEOUT_MS = 30000 // 30 seconds + // Transactional state posting + private uiUpdatePaused: boolean = false + private pendingState: ExtensionState | null = null + public isViewLaunched = false public settingsImportedAt?: number public readonly latestAnnouncementId = "oct-2025-v3.29.0-cloud-agents" // v3.29.0 Cloud Agents announcement @@ -1614,8 +1618,26 @@ export class ClineProvider await this.postStateToWebview() } + public beginStateTransaction(): void { + this.uiUpdatePaused = true + } + + public async endStateTransaction(): Promise { + this.uiUpdatePaused = false + if (this.pendingState) { + await this.view?.webview.postMessage({ type: "state", state: this.pendingState }) + this.pendingState = null + } + } + async postStateToWebview() { const state = await this.getStateToPostToWebview() + + if (this.uiUpdatePaused) { + this.pendingState = state + return + } + this.postMessageToWebview({ type: "state", state }) // Check MDM compliance and send user to account tab if not compliant @@ -2607,24 +2629,13 @@ export class ClineProvider console.log(`[cancelTask] cancelling task ${task.taskId}.${task.instanceId}`) - const { historyItem, uiMessagesFilePath } = await this.getTaskWithId(task.taskId) - - // Preserve parent and root task information for history item. - const rootTask = task.rootTask - const parentTask = task.parentTask - - // Mark this as a user-initiated cancellation so provider-only rehydration can occur + // Mark this as a user-initiated cancellation task.abortReason = "user_cancelled" - // Capture the current instance to detect if rehydrate already occurred elsewhere - const originalInstanceId = task.instanceId - - // Begin abort (non-blocking) - task.abortTask() - - // Immediately mark the original instance as abandoned to prevent any residual activity - task.abandoned = true + // Soft abort (isAbandoned = false) to keep the instance alive + await task.abortTask(false) + // Wait for abort to complete await pWaitFor( () => this.getCurrentTask()! === undefined || @@ -2641,28 +2652,49 @@ export class ClineProvider console.error("Failed to abort task") }) - // Defensive safeguard: if current instance already changed, skip rehydrate - const current = this.getCurrentTask() - if (current && current.instanceId !== originalInstanceId) { - this.log( - `[cancelTask] Skipping rehydrate: current instance ${current.instanceId} != original ${originalInstanceId}`, - ) - return - } + // Deterministic spinner stop: If the last api_req_started has no cost and no cancelReason, + // inject cancelReason to stop the spinner + try { + let lastApiReqStartedIndex = -1 + for (let i = task.clineMessages.length - 1; i >= 0; i--) { + if (task.clineMessages[i].type === "say" && task.clineMessages[i].say === "api_req_started") { + lastApiReqStartedIndex = i + break + } + } - // Final race check before rehydrate to avoid duplicate rehydration - { - const currentAfterCheck = this.getCurrentTask() - if (currentAfterCheck && currentAfterCheck.instanceId !== originalInstanceId) { - this.log( - `[cancelTask] Skipping rehydrate after final check: current instance ${currentAfterCheck.instanceId} != original ${originalInstanceId}`, - ) - return + if (lastApiReqStartedIndex !== -1) { + const lastApiReqStarted = task.clineMessages[lastApiReqStartedIndex] + const apiReqInfo = JSON.parse(lastApiReqStarted.text || "{}") + + if (apiReqInfo.cost === undefined && apiReqInfo.cancelReason === undefined) { + apiReqInfo.cancelReason = "user_cancelled" + lastApiReqStarted.text = JSON.stringify(apiReqInfo) + await task.overwriteClineMessages([...task.clineMessages]) + console.log(`[cancelTask] Injected cancelReason for deterministic spinner stop`) + } } + } catch (error) { + console.error(`[cancelTask] Failed to inject cancelReason:`, error) } - // Clears task again, so we need to abortTask manually above. - await this.createTaskWithHistoryItem({ ...historyItem, rootTask, parentTask }) + // Update UI immediately to reflect current state + await this.postStateToWebview() + + // Schedule non-blocking resumption to present "Resume Task" ask + // Use setImmediate to avoid blocking the webview handler + setImmediate(() => { + if (task && !task.abandoned) { + // Present a resume ask without rehydrating - just show the Resume/Terminate UI + task.presentResumableAsk().catch((error) => { + console.error( + `[cancelTask] Failed to present resume ask: ${ + error instanceof Error ? error.message : String(error) + }`, + ) + }) + } + }) } // Clear the current task without treating it as a subtask. diff --git a/src/core/webview/__tests__/ClineProvider.cancelTask.present-ask.spec.ts b/src/core/webview/__tests__/ClineProvider.cancelTask.present-ask.spec.ts new file mode 100644 index 00000000000..f89dcc4c5ad --- /dev/null +++ b/src/core/webview/__tests__/ClineProvider.cancelTask.present-ask.spec.ts @@ -0,0 +1,60 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest" +import { ClineProvider } from "../ClineProvider" + +describe("ClineProvider.cancelTask - schedules presentResumableAsk", () => { + let provider: ClineProvider + let mockTask: any + + beforeEach(() => { + vi.useFakeTimers() + // Create a bare instance without running constructor + provider = Object.create(ClineProvider.prototype) as ClineProvider + + mockTask = { + taskId: "task-1", + instanceId: "inst-1", + abortReason: undefined, + abandoned: false, + abortTask: vi.fn().mockResolvedValue(undefined), + isStreaming: false, + didFinishAbortingStream: true, + isWaitingForFirstChunk: false, + // Last api_req_started without cost/cancelReason so provider injects cancelReason + clineMessages: [ + { ts: Date.now() - 2000, type: "say", say: "text", text: "hello" }, + { ts: Date.now() - 1000, type: "say", say: "api_req_started", text: JSON.stringify({}) }, + ], + overwriteClineMessages: vi.fn().mockResolvedValue(undefined), + presentResumableAsk: vi.fn().mockResolvedValue(undefined), + } + + // Patch required instance methods used by cancelTask + ;(provider as any).getCurrentTask = vi.fn().mockReturnValue(mockTask) + ;(provider as any).postStateToWebview = vi.fn().mockResolvedValue(undefined) + }) + + afterEach(() => { + vi.useRealTimers() + }) + + it("injects cancelReason and schedules presentResumableAsk on soft cancel", async () => { + // Act + await (provider as any).cancelTask() + + // Assert that abort was initiated + expect(mockTask.abortTask).toHaveBeenCalledWith(false) + + // cancelReason injected for spinner stop + const last = mockTask.clineMessages.at(-1) + expect(last.say).toBe("api_req_started") + const parsed = JSON.parse(last.text || "{}") + expect(parsed.cancelReason).toBe("user_cancelled") + + // presentResumableAsk is scheduled via setImmediate + expect(mockTask.presentResumableAsk).not.toHaveBeenCalled() + vi.runAllTimers() + // run microtasks as well to flush any pending promises in the scheduled callback + await Promise.resolve() + expect(mockTask.presentResumableAsk).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index c409f15a65d..7c386a4e872 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -1027,18 +1027,26 @@ export const webviewMessageHandler = async ( const result = checkoutRestorePayloadSchema.safeParse(message.payload) if (result.success) { - await provider.cancelTask() + // Begin transaction to buffer state updates + provider.beginStateTransaction() try { - await pWaitFor(() => provider.getCurrentTask()?.isInitialized === true, { timeout: 3_000 }) - } catch (error) { - vscode.window.showErrorMessage(t("common:errors.checkpoint_timeout")) - } + await provider.cancelTask() - try { - await provider.getCurrentTask()?.checkpointRestore(result.data) - } catch (error) { - vscode.window.showErrorMessage(t("common:errors.checkpoint_failed")) + try { + await pWaitFor(() => provider.getCurrentTask()?.isInitialized === true, { timeout: 3_000 }) + } catch (error) { + vscode.window.showErrorMessage(t("common:errors.checkpoint_timeout")) + } + + try { + await provider.getCurrentTask()?.checkpointRestore(result.data) + } catch (error) { + vscode.window.showErrorMessage(t("common:errors.checkpoint_failed")) + } + } finally { + // End transaction and post consolidated state + await provider.endStateTransaction() } } diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index 0683f2ebd03..00c5b8bc3ac 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -547,9 +547,15 @@ const ChatViewComponent: React.ForwardRefRenderFunction Date: Mon, 3 Nov 2025 09:11:58 -0500 Subject: [PATCH 2/5] fix: replace JSON.parse with safeJsonParse for improved error handling --- webview-ui/src/components/chat/ChatView.tsx | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index 00c5b8bc3ac..42c93fa5b6b 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -58,6 +58,7 @@ import { QueuedMessages } from "./QueuedMessages" import DismissibleUpsell from "../common/DismissibleUpsell" import { useCloudUpsell } from "@src/hooks/useCloudUpsell" import { Cloud } from "lucide-react" +import { safeJsonParse } from "../../../../src/shared/safeJsonParse" export interface ChatViewProps { isHidden: boolean @@ -547,16 +548,18 @@ const ChatViewComponent: React.ForwardRefRenderFunction Date: Mon, 3 Nov 2025 10:55:21 -0500 Subject: [PATCH 3/5] fix: correct isStreaming logic when cost is defined The logic was inverted - when cost is defined in api_req_started, it means the API request has finished (streaming is complete), so should return false, not true. --- webview-ui/src/components/chat/ChatView.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index 42c93fa5b6b..0aab84125c2 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -558,7 +558,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction Date: Mon, 3 Nov 2025 11:24:06 -0500 Subject: [PATCH 4/5] fix: properly handle streaming state when api_req_started has no cost When api_req_started exists without cost or cancelReason fields, the API request is still in progress and should be treated as streaming. Only return false when cost (completed) or cancelReason (cancelled) are present. --- webview-ui/src/components/chat/ChatView.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/webview-ui/src/components/chat/ChatView.tsx b/webview-ui/src/components/chat/ChatView.tsx index 0aab84125c2..328fb3f7f4e 100644 --- a/webview-ui/src/components/chat/ChatView.tsx +++ b/webview-ui/src/components/chat/ChatView.tsx @@ -560,6 +560,9 @@ const ChatViewComponent: React.ForwardRefRenderFunction Date: Mon, 3 Nov 2025 15:23:27 -0500 Subject: [PATCH 5/5] refactor: centralize abort/streaming state reset logic and use safe JSON parsing - Extract duplicated abort/streaming state reset logic into private helper method resetAbortAndStreamingState() - Update resumeTaskFromHistory() and presentResumableAsk() to use the helper method - Replace unsafe JSON.parse() with safeJsonParse() in ClineProvider.ts for better error handling Addresses review feedback from @mrubens on PR #8986 --- src/core/task/Task.ts | 65 ++++++++++++++----------------- src/core/webview/ClineProvider.ts | 8 +++- 2 files changed, 35 insertions(+), 38 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 9c44bbb9561..03dc3784e29 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -1353,24 +1353,7 @@ export class Task extends EventEmitter implements TaskLike { // (abandoned = false), we keep the instance alive but set abort = true. // We only clear these flags after the user confirms they want to resume, // preventing the old stream from continuing if abort was set. - this.abort = false - this.abandoned = false - this.abortReason = undefined - this.didFinishAbortingStream = false - this.isStreaming = false - - // Reset streaming-local fields to avoid stale state from previous stream - this.currentStreamingContentIndex = 0 - this.currentStreamingDidCheckpoint = false - this.assistantMessageContent = [] - this.didCompleteReadingStream = false - this.userMessageContent = [] - this.userMessageContentReady = false - this.didRejectTool = false - this.didAlreadyUseTool = false - this.presentAssistantMessageLocked = false - this.presentAssistantMessageHasPendingUpdates = false - this.assistantMessageParser.reset() + this.resetAbortAndStreamingState() let responseText: string | undefined let responseImages: string[] | undefined @@ -1550,6 +1533,33 @@ export class Task extends EventEmitter implements TaskLike { await this.initiateTaskLoop(newUserContent) } + /** + * Resets abort flags and streaming state to allow task resumption. + * Centralizes the state reset logic used after user confirms task resumption. + * + * @private + */ + private resetAbortAndStreamingState(): void { + this.abort = false + this.abandoned = false + this.abortReason = undefined + this.didFinishAbortingStream = false + this.isStreaming = false + + // Reset streaming-local fields to avoid stale state from previous stream + this.currentStreamingContentIndex = 0 + this.currentStreamingDidCheckpoint = false + this.assistantMessageContent = [] + this.didCompleteReadingStream = false + this.userMessageContent = [] + this.userMessageContentReady = false + this.didRejectTool = false + this.didAlreadyUseTool = false + this.presentAssistantMessageLocked = false + this.presentAssistantMessageHasPendingUpdates = false + this.assistantMessageParser.reset() + } + /** * Present a resumable ask on an aborted task without rehydrating. * Used by soft-interrupt (cancelTask) to show Resume/Terminate UI. @@ -1574,24 +1584,7 @@ export class Task extends EventEmitter implements TaskLike { // If user clicked Resume (not Terminate), reset abort flags and continue if (response === "yesButtonClicked" || response === "messageResponse") { // Reset abort flags to allow the loop to continue - this.abort = false - this.abandoned = false - this.abortReason = undefined - this.didFinishAbortingStream = false - this.isStreaming = false - - // Reset streaming-local fields to avoid stale state from previous stream - this.currentStreamingContentIndex = 0 - this.currentStreamingDidCheckpoint = false - this.assistantMessageContent = [] - this.didCompleteReadingStream = false - this.userMessageContent = [] - this.userMessageContentReady = false - this.didRejectTool = false - this.didAlreadyUseTool = false - this.presentAssistantMessageLocked = false - this.presentAssistantMessageHasPendingUpdates = false - this.assistantMessageParser.reset() + this.resetAbortAndStreamingState() // Prepare content for resuming the task loop let userContent: Anthropic.Messages.ContentBlockParam[] = [] diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 8878ba35af7..684c51764cb 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -50,6 +50,7 @@ import { Package } from "../../shared/package" import { findLast } from "../../shared/array" import { supportPrompt } from "../../shared/support-prompt" import { GlobalFileNames } from "../../shared/globalFileNames" +import { safeJsonParse } from "../../shared/safeJsonParse" import type { ExtensionMessage, ExtensionState, MarketplaceInstalledMetadata } from "../../shared/ExtensionMessage" import { Mode, defaultModeSlug, getModeBySlug } from "../../shared/modes" import { experimentDefault } from "../../shared/experiments" @@ -2665,9 +2666,12 @@ export class ClineProvider if (lastApiReqStartedIndex !== -1) { const lastApiReqStarted = task.clineMessages[lastApiReqStartedIndex] - const apiReqInfo = JSON.parse(lastApiReqStarted.text || "{}") + const apiReqInfo = safeJsonParse<{ cost?: number; cancelReason?: string }>( + lastApiReqStarted.text || "{}", + {}, + ) - if (apiReqInfo.cost === undefined && apiReqInfo.cancelReason === undefined) { + if (apiReqInfo && apiReqInfo.cost === undefined && apiReqInfo.cancelReason === undefined) { apiReqInfo.cancelReason = "user_cancelled" lastApiReqStarted.text = JSON.stringify(apiReqInfo) await task.overwriteClineMessages([...task.clineMessages])