Skip to content

Commit 58edc71

Browse files
authored
fix: prevent UI flicker and enable resumption after task cancellation (#8986)
1 parent 5c738de commit 58edc71

File tree

7 files changed

+411
-56
lines changed

7 files changed

+411
-56
lines changed

src/core/task/Task.ts

Lines changed: 100 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -737,7 +737,8 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
737737
// deallocated. (Although we set Cline = undefined in provider, that
738738
// simply removes the reference to this instance, but the instance is
739739
// still alive until this promise resolves or rejects.)
740-
if (this.abort) {
740+
// Exception: Allow resume asks even when aborted for soft-interrupt UX
741+
if (this.abort && type !== "resume_task" && type !== "resume_completed_task") {
741742
throw new Error(`[RooCode#ask] task ${this.taskId}.${this.instanceId} aborted`)
742743
}
743744

@@ -1255,7 +1256,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
12551256
])
12561257
}
12571258

1258-
private async resumeTaskFromHistory() {
1259+
public async resumeTaskFromHistory() {
12591260
if (this.enableBridge) {
12601261
try {
12611262
await BridgeOrchestrator.subscribeToTask(this)
@@ -1347,6 +1348,13 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
13471348

13481349
const { response, text, images } = await this.ask(askType) // Calls `postStateToWebview`.
13491350

1351+
// Reset abort flags AFTER user responds to resume ask.
1352+
// This is critical for the cancel → resume flow: when a task is soft-aborted
1353+
// (abandoned = false), we keep the instance alive but set abort = true.
1354+
// We only clear these flags after the user confirms they want to resume,
1355+
// preventing the old stream from continuing if abort was set.
1356+
this.resetAbortAndStreamingState()
1357+
13501358
let responseText: string | undefined
13511359
let responseImages: string[] | undefined
13521360

@@ -1525,6 +1533,86 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
15251533
await this.initiateTaskLoop(newUserContent)
15261534
}
15271535

1536+
/**
1537+
* Resets abort flags and streaming state to allow task resumption.
1538+
* Centralizes the state reset logic used after user confirms task resumption.
1539+
*
1540+
* @private
1541+
*/
1542+
private resetAbortAndStreamingState(): void {
1543+
this.abort = false
1544+
this.abandoned = false
1545+
this.abortReason = undefined
1546+
this.didFinishAbortingStream = false
1547+
this.isStreaming = false
1548+
1549+
// Reset streaming-local fields to avoid stale state from previous stream
1550+
this.currentStreamingContentIndex = 0
1551+
this.currentStreamingDidCheckpoint = false
1552+
this.assistantMessageContent = []
1553+
this.didCompleteReadingStream = false
1554+
this.userMessageContent = []
1555+
this.userMessageContentReady = false
1556+
this.didRejectTool = false
1557+
this.didAlreadyUseTool = false
1558+
this.presentAssistantMessageLocked = false
1559+
this.presentAssistantMessageHasPendingUpdates = false
1560+
this.assistantMessageParser.reset()
1561+
}
1562+
1563+
/**
1564+
* Present a resumable ask on an aborted task without rehydrating.
1565+
* Used by soft-interrupt (cancelTask) to show Resume/Terminate UI.
1566+
* Selects the appropriate ask type based on the last relevant message.
1567+
* If the user clicks Resume, resets abort flags and continues the task loop.
1568+
*/
1569+
public async presentResumableAsk(): Promise<void> {
1570+
const lastClineMessage = this.clineMessages
1571+
.slice()
1572+
.reverse()
1573+
.find((m) => !(m.ask === "resume_task" || m.ask === "resume_completed_task"))
1574+
1575+
let askType: ClineAsk
1576+
if (lastClineMessage?.ask === "completion_result") {
1577+
askType = "resume_completed_task"
1578+
} else {
1579+
askType = "resume_task"
1580+
}
1581+
1582+
const { response, text, images } = await this.ask(askType)
1583+
1584+
// If user clicked Resume (not Terminate), reset abort flags and continue
1585+
if (response === "yesButtonClicked" || response === "messageResponse") {
1586+
// Reset abort flags to allow the loop to continue
1587+
this.resetAbortAndStreamingState()
1588+
1589+
// Prepare content for resuming the task loop
1590+
let userContent: Anthropic.Messages.ContentBlockParam[] = []
1591+
1592+
if (response === "messageResponse" && text) {
1593+
// User provided additional instructions
1594+
await this.say("user_feedback", text, images)
1595+
userContent.push({
1596+
type: "text",
1597+
text: `\n\nNew instructions for task continuation:\n<user_message>\n${text}\n</user_message>`,
1598+
})
1599+
if (images && images.length > 0) {
1600+
userContent.push(...formatResponse.imageBlocks(images))
1601+
}
1602+
} else {
1603+
// Simple resume with no new instructions
1604+
userContent.push({
1605+
type: "text",
1606+
text: "[TASK RESUMPTION] Resuming task...",
1607+
})
1608+
}
1609+
1610+
// Continue the task loop
1611+
await this.initiateTaskLoop(userContent)
1612+
}
1613+
// If user clicked Terminate (noButtonClicked), do nothing - task stays aborted
1614+
}
1615+
15281616
public async abortTask(isAbandoned = false) {
15291617
// Aborting task
15301618

@@ -1536,12 +1624,17 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
15361624
this.abort = true
15371625
this.emit(RooCodeEventName.TaskAborted)
15381626

1539-
try {
1540-
this.dispose() // Call the centralized dispose method
1541-
} catch (error) {
1542-
console.error(`Error during task ${this.taskId}.${this.instanceId} disposal:`, error)
1543-
// Don't rethrow - we want abort to always succeed
1627+
// Only dispose if this is a hard abort (abandoned)
1628+
// For soft abort (user cancel), keep the instance alive so we can present a resumable ask
1629+
if (isAbandoned) {
1630+
try {
1631+
this.dispose() // Call the centralized dispose method
1632+
} catch (error) {
1633+
console.error(`Error during task ${this.taskId}.${this.instanceId} disposal:`, error)
1634+
// Don't rethrow - we want abort to always succeed
1635+
}
15441636
}
1637+
15451638
// Save the countdown message in the automatic retry or other content.
15461639
try {
15471640
// Save the countdown message in the automatic retry or other content.
Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"
2+
import { ProviderSettings } from "@roo-code/types"
3+
4+
import { Task } from "../Task"
5+
import { ClineProvider } from "../../webview/ClineProvider"
6+
7+
// Mocks similar to Task.dispose.test.ts
8+
vi.mock("../../webview/ClineProvider")
9+
vi.mock("../../../integrations/terminal/TerminalRegistry", () => ({
10+
TerminalRegistry: {
11+
releaseTerminalsForTask: vi.fn(),
12+
},
13+
}))
14+
vi.mock("../../ignore/RooIgnoreController")
15+
vi.mock("../../protect/RooProtectedController")
16+
vi.mock("../../context-tracking/FileContextTracker")
17+
vi.mock("../../../services/browser/UrlContentFetcher")
18+
vi.mock("../../../services/browser/BrowserSession")
19+
vi.mock("../../../integrations/editor/DiffViewProvider")
20+
vi.mock("../../tools/ToolRepetitionDetector")
21+
vi.mock("../../../api", () => ({
22+
buildApiHandler: vi.fn(() => ({
23+
getModel: () => ({ info: {}, id: "test-model" }),
24+
})),
25+
}))
26+
vi.mock("../AutoApprovalHandler")
27+
28+
// Mock TelemetryService
29+
vi.mock("@roo-code/telemetry", () => ({
30+
TelemetryService: {
31+
instance: {
32+
captureTaskCreated: vi.fn(),
33+
captureTaskRestarted: vi.fn(),
34+
captureConversationMessage: vi.fn(),
35+
},
36+
},
37+
}))
38+
39+
describe("Task.presentResumableAsk abort reset", () => {
40+
let mockProvider: any
41+
let mockApiConfiguration: ProviderSettings
42+
let task: Task
43+
44+
beforeEach(() => {
45+
vi.clearAllMocks()
46+
47+
mockProvider = {
48+
context: {
49+
globalStorageUri: { fsPath: "/test/path" },
50+
},
51+
getState: vi.fn().mockResolvedValue({ mode: "code" }),
52+
postStateToWebview: vi.fn().mockResolvedValue(undefined),
53+
postMessageToWebview: vi.fn().mockResolvedValue(undefined),
54+
updateTaskHistory: vi.fn().mockResolvedValue(undefined),
55+
log: vi.fn(),
56+
}
57+
58+
mockApiConfiguration = {
59+
apiProvider: "anthropic",
60+
apiKey: "test-key",
61+
} as ProviderSettings
62+
63+
task = new Task({
64+
provider: mockProvider as ClineProvider,
65+
apiConfiguration: mockApiConfiguration,
66+
startTask: false,
67+
})
68+
})
69+
70+
afterEach(() => {
71+
// Ensure we don't leave event listeners dangling
72+
task.dispose()
73+
})
74+
75+
it("resets abort flags and continues the loop on yesButtonClicked", async () => {
76+
// Arrange aborted state
77+
task.abort = true
78+
task.abortReason = "user_cancelled"
79+
task.didFinishAbortingStream = true
80+
task.isStreaming = true
81+
82+
// minimal message history
83+
task.clineMessages = [{ ts: Date.now() - 1000, type: "say", say: "text", text: "prev" } as any]
84+
85+
// Spy and stub ask + loop
86+
const askSpy = vi.spyOn(task as any, "ask").mockResolvedValue({ response: "yesButtonClicked" })
87+
const loopSpy = vi.spyOn(task as any, "initiateTaskLoop").mockResolvedValue(undefined)
88+
89+
// Act
90+
await task.presentResumableAsk()
91+
92+
// Assert ask was presented
93+
expect(askSpy).toHaveBeenCalled()
94+
95+
// Abort flags cleared
96+
expect(task.abort).toBe(false)
97+
expect(task.abandoned).toBe(false)
98+
expect(task.abortReason).toBeUndefined()
99+
expect(task.didFinishAbortingStream).toBe(false)
100+
expect(task.isStreaming).toBe(false)
101+
102+
// Streaming-local state cleared
103+
expect(task.currentStreamingContentIndex).toBe(0)
104+
expect(task.assistantMessageContent).toEqual([])
105+
expect(task.userMessageContentReady).toBe(false)
106+
expect(task.didRejectTool).toBe(false)
107+
expect(task.presentAssistantMessageLocked).toBe(false)
108+
109+
// Loop resumed
110+
expect(loopSpy).toHaveBeenCalledTimes(1)
111+
})
112+
113+
it("includes user feedback when resuming with messageResponse", async () => {
114+
task.abort = true
115+
task.clineMessages = [{ ts: Date.now() - 1000, type: "say", say: "text", text: "prev" } as any]
116+
117+
const askSpy = vi
118+
.spyOn(task as any, "ask")
119+
.mockResolvedValue({ response: "messageResponse", text: "Continue with this", images: undefined })
120+
const saySpy = vi.spyOn(task, "say").mockResolvedValue(undefined as any)
121+
const loopSpy = vi.spyOn(task as any, "initiateTaskLoop").mockResolvedValue(undefined)
122+
123+
await task.presentResumableAsk()
124+
125+
expect(askSpy).toHaveBeenCalled()
126+
expect(saySpy).toHaveBeenCalledWith("user_feedback", "Continue with this", undefined)
127+
expect(loopSpy).toHaveBeenCalledTimes(1)
128+
})
129+
130+
it("does nothing when user clicks Terminate (noButtonClicked)", async () => {
131+
task.abort = true
132+
task.abortReason = "user_cancelled"
133+
task.clineMessages = [{ ts: Date.now() - 1000, type: "say", say: "text", text: "prev" } as any]
134+
135+
vi.spyOn(task as any, "ask").mockResolvedValue({ response: "noButtonClicked" })
136+
const loopSpy = vi.spyOn(task as any, "initiateTaskLoop").mockResolvedValue(undefined)
137+
138+
await task.presentResumableAsk()
139+
140+
// Still aborted
141+
expect(task.abort).toBe(true)
142+
expect(task.abortReason).toBe("user_cancelled")
143+
// No loop resume
144+
expect(loopSpy).not.toHaveBeenCalled()
145+
})
146+
})

src/core/task/__tests__/Task.spec.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1722,12 +1722,12 @@ describe("Cline", () => {
17221722
// Mock the dispose method to track cleanup
17231723
const disposeSpy = vi.spyOn(task, "dispose").mockImplementation(() => {})
17241724

1725-
// Call abortTask
1725+
// Call abortTask (soft cancel - same path as UI Cancel button)
17261726
await task.abortTask()
17271727

1728-
// Verify the same behavior as Cancel button
1728+
// Verify the same behavior as Cancel button: soft abort sets abort flag but does not dispose
17291729
expect(task.abort).toBe(true)
1730-
expect(disposeSpy).toHaveBeenCalled()
1730+
expect(disposeSpy).not.toHaveBeenCalled()
17311731
})
17321732

17331733
it("should work with TaskLike interface", async () => {
@@ -1771,8 +1771,8 @@ describe("Cline", () => {
17711771
// Spy on console.error to verify error is logged
17721772
const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {})
17731773

1774-
// abortTask should not throw even if dispose fails
1775-
await expect(task.abortTask()).resolves.not.toThrow()
1774+
// abortTask should not throw even if dispose fails (hard abort triggers dispose)
1775+
await expect(task.abortTask(true)).resolves.not.toThrow()
17761776

17771777
// Verify error was logged
17781778
expect(consoleErrorSpy).toHaveBeenCalledWith(expect.stringContaining("Error during task"), mockError)

0 commit comments

Comments
 (0)