Skip to content

Commit 7b829fa

Browse files
committed
fix(task): apply first-reason-wins to both abort paths in catch block
1 parent 62b822f commit 7b829fa

3 files changed

Lines changed: 39 additions & 4 deletions

File tree

‎src/core/task/Task.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3751,8 +3751,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
37513751
console.log(
37523752
`[Task#${this.taskId}.${this.instanceId}] Task aborted during mid-stream retry backoff`,
37533753
)
3754-
// Abort the entire task
3755-
this.abortReason = "user_cancelled"
3754+
this.abortReason ??= "user_cancelled"
37563755
await this.abortTask()
37573756
break
37583757
}

‎src/core/task/__tests__/Task.abort-reason-race.spec.ts‎

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ function makeMockProvider() {
120120
}
121121
}
122122

123-
describe("Task abort-reason race (RSK-19 regression)", () => {
123+
describe("Task abort-reason race", () => {
124124
let mockApiConfig: ProviderSettings
125125

126126
beforeEach(() => {
@@ -173,6 +173,42 @@ describe("Task abort-reason race (RSK-19 regression)", () => {
173173
// The outer catch in recursivelyMakeClineRequests swallows the abort throw and returns true.
174174
const result = await task.recursivelyMakeClineRequests([{ type: "text", text: "help me" }])
175175
expect(result).toBe(true)
176+
expect(task.didFinishAbortingStream).toBe(true)
177+
expect(abortTaskSpy).toHaveBeenCalledOnce()
178+
expect(task.abortReason).toBe("user_cancelled")
179+
})
180+
181+
it("keeps pre-existing abortReason when cancel lands during retry backoff", async () => {
182+
const mockProvider = makeMockProvider()
183+
// autoApprovalEnabled causes the else branch to call backoffAndAnnounce.
184+
mockProvider.getState = vi.fn().mockResolvedValue({ autoApprovalEnabled: true })
185+
186+
// Double cast: plain object satisfies only the methods called by this code path.
187+
const task = new Task({
188+
provider: mockProvider as unknown as ClineProvider,
189+
apiConfiguration: mockApiConfig,
190+
task: "test task",
191+
startTask: false,
192+
})
193+
194+
vi.spyOn(task.diffViewProvider, "reset").mockResolvedValue(undefined)
195+
vi.spyOn(task, "dispose").mockResolvedValue(undefined)
196+
197+
// eslint-disable-next-line require-yield -- intentional error-only async source
198+
task["attemptApiRequest"] = async function* () {
199+
throw new Error("mid-stream failure")
200+
}
201+
202+
// Simulate cancel landing during backoff: cancelTask sets abortReason before abort=true.
203+
task["backoffAndAnnounce"] = async () => {
204+
task["abortReason"] = "user_cancelled"
205+
task["abort"] = true
206+
}
207+
208+
const abortTaskSpy = vi.spyOn(task, "abortTask")
209+
// break after backoff exits the while loop; the outer try returns false.
210+
const result = await task.recursivelyMakeClineRequests([{ type: "text", text: "help me" }])
211+
expect(result).toBe(false)
176212
expect(abortTaskSpy).toHaveBeenCalledOnce()
177213
expect(task.abortReason).toBe("user_cancelled")
178214
})

‎src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -961,7 +961,7 @@ describe("ClineProvider Task History Synchronization", () => {
961961
expect(logSpy).toHaveBeenCalledWith(expect.stringContaining("[onTaskCompleted] Failed to write"))
962962
})
963963

964-
it("onTaskAborted does not call createTaskWithHistoryItem (RSK-19 guard)", async () => {
964+
it("onTaskAborted does not call createTaskWithHistoryItem", async () => {
965965
// Store the item so getTaskWithId succeeds: without it, the old branch's catch
966966
// swallows the error and the test passes even if the branch comes back.
967967
const existing = createHistoryItem({ id: "task-abort-1", task: "T" })

0 commit comments

Comments
 (0)