merge: sync main and adapt vector-store tests to readonly config

Author: WebMad

src/core/task/Task.ts

@@ -3236,6 +3236,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 				this.didRejectTool = false
 				this.didAlreadyUseTool = false
 				this.assistantMessageSavedToHistory = false
+				this.didFinishAbortingStream = false
 				this.resetAssistantMessagePersistence()
 				// Reset tool failure flag for each new assistant turn - this ensures that tool failures
 				// only prevent attempt_completion within the same assistant message, not across turns
@@ -3715,8 +3716,8 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 						await abortStream(cancelReason, streamingFailedMessage)
 
 						if (this.abort) {
-							// User cancelled - abort the entire task
-							this.abortReason = cancelReason
+							// ??= keeps the first reason; a cancel can land during abortStream after cancelReason was already computed.
+							this.abortReason ??= "user_cancelled"
 							await this.abortTask()
 						} else if (error instanceof OutputTokenLimitError) {
 							// Truncation repeats on an identical request, so never auto-retry it
@@ -3751,8 +3752,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 									console.log(
 										`[Task#${this.taskId}.${this.instanceId}] Task aborted during mid-stream retry backoff`,
 									)
-									// Abort the entire task
-									this.abortReason = "user_cancelled"
+									this.abortReason ??= "user_cancelled"
 									await this.abortTask()
 									break
 								}

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

@@ -0,0 +1,216 @@
+// npx vitest run core/task/__tests__/Task.abort-reason-race.spec.ts
+
+import * as os from "os"
+import * as path from "path"
+
+import type { GlobalState, ProviderSettings } from "@roo-code/types"
+import { providerIdentifiers } from "@roo-code/types/provider-identifiers"
+import { findLast } from "../../../shared/array"
+import { TelemetryService } from "@roo-code/telemetry"
+
+import { Task } from "../Task"
+import { ClineProvider } from "../../webview/ClineProvider"
+
+const { mockSaveTaskMessages, mockSaveApiMessages } = vi.hoisted(() => ({
+	mockSaveTaskMessages: vi.fn().mockResolvedValue(undefined),
+	mockSaveApiMessages: vi.fn().mockResolvedValue(undefined),
+}))
+
+// vscode is globally aliased to __mocks__/vscode.js by vitest.config.ts.
+vi.mock("delay", () => ({ __esModule: true, default: vi.fn().mockResolvedValue(undefined) }))
+vi.mock("execa", () => ({ execa: vi.fn() }))
+vi.mock("p-wait-for", () => ({ default: vi.fn().mockResolvedValue(undefined) }))
+
+vi.mock("../../task-persistence", async (importOriginal) => {
+	const mod = await importOriginal<typeof import("../../task-persistence")>()
+	return {
+		...mod,
+		saveApiMessages: mockSaveApiMessages,
+		saveTaskMessages: mockSaveTaskMessages,
+		TaskHistoryStore: vi.fn().mockImplementation(function () {
+			return {
+				initialize: vi.fn().mockResolvedValue(undefined),
+				dispose: vi.fn(),
+				get: vi.fn(),
+				getAll: vi.fn().mockReturnValue([]),
+				upsert: vi.fn().mockResolvedValue([]),
+				delete: vi.fn().mockResolvedValue(undefined),
+				deleteMany: vi.fn().mockResolvedValue(undefined),
+				reconcile: vi.fn().mockResolvedValue(undefined),
+				initialized: Promise.resolve(),
+			}
+		}),
+	}
+})
+
+vi.mock("../../mentions", () => ({
+	parseMentions: vi
+		.fn()
+		.mockImplementation((text) =>
+			Promise.resolve({ text: `processed: ${text}`, mode: undefined, contentBlocks: [] }),
+		),
+	openMention: vi.fn(),
+	getLatestTerminalOutput: vi.fn(),
+}))
+
+vi.mock("../../mentions/processUserContentMentions", () => ({
+	processUserContentMentions: vi.fn().mockImplementation(async ({ userContent }: { userContent: unknown[] }) => ({
+		content: userContent,
+		mode: undefined,
+	})),
+}))
+
+vi.mock("../../../integrations/misc/extract-text", () => ({
+	extractTextFromFile: vi.fn().mockResolvedValue(""),
+}))
+
+vi.mock("../../environment/getEnvironmentDetails", () => ({
+	getEnvironmentDetails: vi.fn().mockResolvedValue(""),
+}))
+
+vi.mock("../../ignore/RooIgnoreController")
+
+vi.mock("../../../utils/storage", () => ({
+	getTaskDirectoryPath: vi
+		.fn()
+		.mockImplementation((base: string, id: string) => Promise.resolve(`${base}/tasks/${id}`)),
+	getSettingsDirectoryPath: vi.fn().mockImplementation((base: string) => Promise.resolve(`${base}/settings`)),
+}))
+
+vi.mock("../../../utils/fs", () => ({
+	fileExistsAtPath: vi.fn().mockReturnValue(false),
+}))
+
+vi.mock("../../../i18n", () => ({
+	t: (key: string) => key,
+}))
+
+function makeMockProvider() {
+	return {
+		log: vi.fn(),
+		taskHistoryStore: { get: () => undefined },
+		updateTaskHistory: vi.fn().mockResolvedValue([]),
+		getState: vi.fn().mockResolvedValue({}),
+		getSkillsManager: vi.fn().mockReturnValue(undefined),
+		postStateToWebviewWithoutTaskHistory: vi.fn().mockResolvedValue(undefined),
+		flushPostStateToWebviewThrottled: vi.fn().mockResolvedValue(undefined),
+		postMessageToWebview: vi.fn().mockResolvedValue(undefined),
+		postStateToWebview: vi.fn().mockResolvedValue(undefined),
+		postStateToWebviewThrottled: vi.fn().mockResolvedValue(undefined),
+		context: {
+			globalStorageUri: { fsPath: path.join(os.tmpdir(), "test-storage-abort-race") },
+			globalState: {
+				get: vi.fn().mockImplementation((_key: keyof GlobalState) => undefined),
+				update: vi.fn().mockResolvedValue(undefined),
+				keys: vi.fn().mockReturnValue([]),
+			},
+			workspaceState: {
+				get: vi.fn().mockImplementation(() => undefined),
+				update: vi.fn().mockResolvedValue(undefined),
+				keys: vi.fn().mockReturnValue([]),
+			},
+			secrets: {
+				get: vi.fn().mockResolvedValue(undefined),
+				store: vi.fn().mockResolvedValue(undefined),
+				delete: vi.fn().mockResolvedValue(undefined),
+			},
+			extensionUri: { fsPath: "/mock/extension" },
+			extension: { packageJSON: { version: "1.0.0" } },
+		},
+	}
+}
+
+describe("Task abort-reason race", () => {
+	let mockApiConfig: ProviderSettings
+
+	beforeEach(() => {
+		vi.clearAllMocks()
+		mockSaveTaskMessages.mockResolvedValue(undefined)
+
+		if (!TelemetryService.hasInstance()) {
+			TelemetryService.createInstance([])
+		}
+
+		mockApiConfig = {
+			apiProvider: providerIdentifiers.anthropic,
+			apiModelId: "claude-3-5-sonnet-20241022",
+			apiKey: "test-api-key",
+		}
+	})
+
+	it("keeps user_cancelled when a cancel lands during abortStream cleanup", async () => {
+		const mockProvider = makeMockProvider()
+
+		// Double cast: plain object satisfies only the methods called by this code path.
+		const task = new Task({
+			provider: mockProvider as unknown as ClineProvider,
+			apiConfiguration: mockApiConfig,
+			task: "test task",
+			startTask: false,
+		})
+
+		vi.spyOn(task.diffViewProvider, "reset").mockResolvedValue(undefined)
+		vi.spyOn(task, "dispose").mockResolvedValue(undefined)
+		const abortTaskSpy = vi.spyOn(task, "abortTask")
+
+		// eslint-disable-next-line require-yield -- intentional error-only async source
+		task["attemptApiRequest"] = async function* () {
+			throw new Error("mid-stream API failure")
+		}
+
+		// abortStream calls updateApiReqMsg("streaming_failed") synchronously before saving,
+		// so the api_req_started row has cancelReason="streaming_failed" when we are inside it.
+		mockSaveTaskMessages.mockImplementation(async () => {
+			const apiReqMsg = findLast(task.clineMessages, (m) => m.say === "api_req_started")
+			const parsed = apiReqMsg?.text ? (JSON.parse(apiReqMsg.text) as { cancelReason?: string }) : {}
+			if (parsed.cancelReason === "streaming_failed" && !task.didFinishAbortingStream) {
+				// cancelTask sets abortReason before abort=true.
+				task["abortReason"] = "user_cancelled"
+				task["abort"] = true
+			}
+		})
+
+		// The outer catch in recursivelyMakeClineRequests swallows the abort throw and returns true.
+		const result = await task.recursivelyMakeClineRequests([{ type: "text", text: "help me" }])
+		expect(result).toBe(true)
+		expect(task.didFinishAbortingStream).toBe(true)
+		expect(abortTaskSpy).toHaveBeenCalledOnce()
+		expect(task.abortReason).toBe("user_cancelled")
+	})
+
+	it("keeps pre-existing abortReason when cancel lands during retry backoff", async () => {
+		const mockProvider = makeMockProvider()
+		// autoApprovalEnabled causes the else branch to call backoffAndAnnounce.
+		mockProvider.getState = vi.fn().mockResolvedValue({ autoApprovalEnabled: true })
+
+		// Double cast: plain object satisfies only the methods called by this code path.
+		const task = new Task({
+			provider: mockProvider as unknown as ClineProvider,
+			apiConfiguration: mockApiConfig,
+			task: "test task",
+			startTask: false,
+		})
+
+		vi.spyOn(task.diffViewProvider, "reset").mockResolvedValue(undefined)
+		vi.spyOn(task, "dispose").mockResolvedValue(undefined)
+
+		// eslint-disable-next-line require-yield -- intentional error-only async source
+		task["attemptApiRequest"] = async function* () {
+			throw new Error("mid-stream failure")
+		}
+
+		// Simulate cancel landing during backoff: cancelTask sets abortReason before abort=true.
+		// Use a distinct seed value so ??= (keeps it) is distinguishable from = (overwrites).
+		task["backoffAndAnnounce"] = async () => {
+			task["abortReason"] = "streaming_failed"
+			task["abort"] = true
+		}
+
+		const abortTaskSpy = vi.spyOn(task, "abortTask")
+		// break after backoff exits the while loop; the outer try returns false.
+		const result = await task.recursivelyMakeClineRequests([{ type: "text", text: "help me" }])
+		expect(result).toBe(false)
+		expect(abortTaskSpy).toHaveBeenCalledOnce()
+		expect(task.abortReason).toBe("streaming_failed")
+	})
+})

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

@@ -86,6 +86,7 @@ vi.mock("delay", () => ({
 }))
 
 import delay from "delay"
+import pWaitFor from "p-wait-for"
 
 vi.mock("uuid", async (importOriginal) => {
 	const actual = await importOriginal<typeof import("uuid")>()
@@ -713,6 +714,98 @@ describe("Cline", () => {
 			])
 		})
 
+		it("clears didFinishAbortingStream on retry so cancelTask waits for the new stream (#1801)", async () => {
+			const task = new Task({
+				provider: mockProvider,
+				apiConfiguration: mockApiConfig,
+				task: "abort flag reset test",
+				startTask: false,
+			})
+
+			vi.spyOn(task.diffViewProvider, "reset").mockResolvedValue(undefined)
+			vi.spyOn(getTaskTestAccess(task), "safeEnsureModelFetched").mockResolvedValue(stubModelInfo)
+			vi.spyOn(getTaskTestAccess(task), "presentAssistantMessageSafe").mockImplementation(() => {})
+
+			const failingStream = async function* (): AsyncGenerator<ApiStreamChunk> {
+				yield { type: "text", text: "partial" }
+				throw new Error("simulated mid-stream failure")
+			}
+
+			let releaseRetryStream: (() => void) | undefined
+			let markRetryStreamPaused: (() => void) | undefined
+			const retryStreamRelease = new Promise<void>((resolve) => {
+				releaseRetryStream = resolve
+			})
+			const retryStreamPaused = new Promise<void>((resolve) => {
+				markRetryStreamPaused = resolve
+			})
+			const pausedRetryStream = async function* (): AsyncGenerator<ApiStreamChunk> {
+				yield { type: "text", text: "retry" }
+				markRetryStreamPaused?.()
+				await retryStreamRelease
+			}
+
+			const attemptApiRequestSpy = vi
+				.spyOn(task, "attemptApiRequest")
+				.mockImplementationOnce(() => failingStream())
+				.mockImplementationOnce(() => pausedRetryStream())
+				.mockImplementation(() => {
+					throw new Error("stop after retry response")
+				})
+
+			const { default: realPWaitFor } = await vi.importActual<typeof import("p-wait-for")>("p-wait-for")
+			vi.mocked(pWaitFor).mockImplementation(realPWaitFor)
+			let request: Promise<boolean> | undefined
+			let cancel: Promise<void> | undefined
+			try {
+				await mockProvider.addClineToStack(task)
+				request = task.recursivelyMakeClineRequests([{ type: "text", text: "abort flag reset test" }])
+				await Promise.race([
+					retryStreamPaused,
+					request.then(() => {
+						throw new Error("request settled before the retry stream paused")
+					}),
+				])
+
+				const firstApiReq = task.clineMessages.find((message) => message.say === "api_req_started")
+				expect(JSON.parse(firstApiReq?.text ?? "{}")).toMatchObject({ cancelReason: "streaming_failed" })
+				expect(attemptApiRequestSpy).toHaveBeenCalledTimes(2)
+
+				let markCancelWaiting: (() => void) | undefined
+				const cancelWaiting = new Promise<void>((resolve) => {
+					markCancelWaiting = resolve
+				})
+				vi.mocked(pWaitFor).mockImplementation((condition, options) =>
+					realPWaitFor(async () => {
+						const done = await condition()
+						if (!done) markCancelWaiting?.()
+						return done
+					}, options),
+				)
+
+				vi.spyOn(mockProvider, "createTaskWithHistoryItem").mockImplementation((historyItem) =>
+					ClineProvider.prototype.createTaskWithHistoryItem.call(mockProvider, historyItem, {
+						startTask: false,
+					}),
+				)
+
+				let cancelSettled = false
+				cancel = mockProvider.cancelTask().then(() => {
+					cancelSettled = true
+				})
+				await Promise.race([cancelWaiting, cancel])
+				expect(cancelSettled, "cancelTask must wait while the retry stream is live").toBe(false)
+
+				releaseRetryStream?.()
+				await cancel
+				await expect(request).resolves.toBe(true)
+			} finally {
+				releaseRetryStream?.()
+				await Promise.allSettled([request, cancel])
+				vi.mocked(pWaitFor).mockImplementation(async () => {})
+			}
+		})
+
 		it("finalizes MCP tool call using the request-scoped parser state", async () => {
 			const task = new Task({
 				provider: mockProvider,

src/core/webview/ClineProvider.ts

@@ -406,35 +406,7 @@ export class ClineProvider
 				}
 				this.emit(RooCodeEventName.TaskCompleted, taskId, tokenUsage, toolUsage)
 			}
-			const onTaskAborted = async () => {
-				this.emit(RooCodeEventName.TaskAborted, instance.taskId)
-
-				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) {
-							this.log(
-								`[onTaskAborted] Skipping rehydrate: current instance ${current.instanceId} != aborted ${instance.instanceId}`,
-							)
-							return
-						}
-
-						const { historyItem } = await this.getTaskWithId(instance.taskId)
-						const rootTask = instance.rootTask
-						const parentTask = instance.parentTask
-						await this.createTaskWithHistoryItem({ ...historyItem, rootTask, parentTask })
-					}
-				} catch (error) {
-					this.log(
-						`[onTaskAborted] Failed to rehydrate after streaming failure: ${
-							error instanceof Error ? error.message : String(error)
-						}`,
-					)
-				}
-			}
+			const onTaskAborted = () => this.emit(RooCodeEventName.TaskAborted, instance.taskId)
 			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)