refactor(task): read abort and abortReason from run-state latches (#2283)

Author: edelauna

src/core/task/Task.ts

@@ -144,7 +144,7 @@ import { mergeConsecutiveApiMessages } from "./mergeConsecutiveApiMessages"
 import { prepareApiConversationMessage } from "./apiConversationHistory"
 import { shouldAddUserMessageToHistory } from "./messageCounting"
 import { type TaskExecutionContext } from "./providerHandoff"
-import { initialRunState, type Ask, type RunEvent, type RunState } from "./run-state/runState"
+import { abort, abortReason, initialRunState, type Ask, type RunEvent, type RunState } from "./run-state/runState"
 import { applyRunEvent, type RunRejection } from "./run-state/runStateShadow"
 import {
 	divergenceKey,
@@ -392,7 +392,6 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 
 	providerRef: WeakRef<ClineProvider>
 	private readonly globalStoragePath: string
-	abort: boolean = false
 	currentRequestAbortController?: AbortController
 	/**
 	 * Controller for the waiter on an in-flight `ensureModelFetched()` call (see
@@ -410,7 +409,6 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 
 	didFinishAbortingStream = false
 	abandoned = false
-	abortReason?: ClineApiReqCancelReason
 	isInitialized = false
 
 	runState: RunState = initialRunState
@@ -3581,16 +3579,22 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 		this.driveRunEvent({ tag: "completionAccepted", askTs })
 	}
 
+	get abort(): boolean {
+		return abort(this.runState)
+	}
+
+	get abortReason(): ClineApiReqCancelReason | undefined {
+		return abortReason(this.runState)
+	}
+
 	setAbortReason(reason: ClineApiReqCancelReason): void {
-		this.abortReason ??= reason
 		this.driveRunEvent({ tag: "reasonSet", reason })
 	}
 
 	requestAbort(isAbandoned: boolean): void {
 		if (isAbandoned) {
 			this.abandoned = true
 		}
-		this.abort = true
 		this.driveRunEvent({ tag: "abortRequested", abandoned: isAbandoned })
 	}
 
@@ -3600,7 +3604,6 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 	}
 
 	requestDispose(): void {
-		this.abort = true
 		this.driveRunEvent({ tag: "disposeRequested" })
 	}
 

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

@@ -21,6 +21,7 @@ import { ContextProxy } from "../../config/ContextProxy"
 import { providerIdentifiers } from "@roo-code/types/provider-identifiers"
 import { attemptCompletionTool, type AttemptCompletionCallbacks } from "../../tools/AttemptCompletionTool"
 import type { ToolUse } from "../../../shared/tools"
+import { setAbortLatch } from "../../../test-utils/run-state"
 
 type TaskPersistenceAccess = {
 	addToApiConversationHistory: (message: Anthropic.MessageParam) => Promise<void>
@@ -909,7 +910,7 @@ describe("Task persistence", () => {
 				task: "test task",
 				startTask: false,
 			})
-			task.abort = true
+			setAbortLatch(task, true)
 			task.userMessageContent = [{ type: "tool_result", tool_use_id: "tool-1", content: "done" }]
 			const waitForPersistence = vi.spyOn(task, "waitForCurrentAssistantMessagePersistence")
 

src/core/task/__tests__/Task.run-shadow.spec.ts

@@ -101,14 +101,14 @@ describe("Task run-state write helpers", () => {
 		task.requestAbort(false)
 
 		expect([task.abort, task.abandoned]).toEqual([true, false])
-		expect(task.runState.latches).toMatchObject({ abort: true, abandoned: false })
+		expect(task.runState.latches.abandoned).toBe(false)
 	})
 
 	it("requestAbort(true) sets abort and abandoned", () => {
 		task.requestAbort(true)
 
 		expect([task.abort, task.abandoned]).toEqual([true, true])
-		expect(task.runState.latches).toMatchObject({ abort: true, abandoned: true })
+		expect(task.runState.latches.abandoned).toBe(true)
 	})
 
 	it("requestAbandon sets abandoned after an abort", () => {
@@ -124,7 +124,7 @@ describe("Task run-state write helpers", () => {
 		task.requestDispose()
 
 		expect(task.abort).toBe(true)
-		expect(task.runState.latches).toMatchObject({ abort: true, disposed: true })
+		expect(task.runState.latches.disposed).toBe(true)
 	})
 
 	it("drives the ask events", () => {
@@ -149,12 +149,15 @@ describe("Task run-state write helpers", () => {
 		expect(task.runRejections).toEqual([{ event: { tag: "abandonRequested" }, state: initialRunState }])
 	})
 
-	it("keeps the first abort reason in the field and in the kernel", () => {
+	it("has no abort reason before one is set", () => {
+		expect(task.abortReason).toBeUndefined()
+	})
+
+	it("keeps the first abort reason", () => {
 		task.setAbortReason("user_cancelled")
 		task.setAbortReason("streaming_failed")
 
 		expect(task.abortReason).toBe("user_cancelled")
-		expect(task.runState.latches.reason).toBe("user_cancelled")
 		expect(task.runRejections).toEqual([])
 	})
 })

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

@@ -99,6 +99,7 @@ vi.mock("delay", () => ({
 
 import delay from "delay"
 import pWaitFor from "p-wait-for"
+import { setAbortLatch } from "../../../test-utils/run-state"
 
 vi.mock("uuid", async (importOriginal) => {
 	const actual = await importOriginal<typeof import("uuid")>()
@@ -1071,7 +1072,7 @@ describe("Cline", () => {
 			const saySpy = vi.spyOn(task, "say").mockImplementation(async (...args) => {
 				const [type, _text, _images, partial] = args
 				if (type === "text" && partial === false) {
-					task.abort = true
+					setAbortLatch(task, true)
 					throw new Error("presenter text failed while aborting")
 				}
 				return say(...args)
@@ -4922,7 +4923,7 @@ describe("Cline", () => {
 				const abortTaskSpy = vi.spyOn(task, "abortTask").mockResolvedValue(undefined)
 
 				// Test Case 1: Stream failure should NOT abort task
-				task.abort = false
+				setAbortLatch(task, false)
 				task.abandoned = false
 
 				// Simulate the catch block behavior for stream failure
@@ -4943,7 +4944,7 @@ describe("Cline", () => {
 				expect(abortTaskSpy).not.toHaveBeenCalled()
 
 				// Test Case 2: User cancellation SHOULD abort task
-				task.abort = true
+				setAbortLatch(task, true)
 
 				// For user cancellation, abortTask SHOULD be called
 				if (task.abort) {
@@ -7344,7 +7345,7 @@ describe("Cline", () => {
 			await flushMicrotasks()
 			consoleErrorSpy.mockClear()
 
-			task.abort = true
+			setAbortLatch(task, true)
 			getTaskTestAccess(task).presentAssistantMessageSafe()
 			await flushMicrotasks()
 
@@ -7398,7 +7399,7 @@ describe("Cline", () => {
 			consoleErrorSpy.mockClear()
 
 			// Simulate the TOCTOU race: abort flips between throw and catch.
-			task.abort = true
+			setAbortLatch(task, true)
 			getTaskTestAccess(task).presentAssistantMessageSafe()
 			await flushMicrotasks()
 

src/core/task/__tests__/ask-abort-race.spec.ts

@@ -3,6 +3,7 @@ import type { ClineMessage } from "@roo-code/types"
 import { checkAutoApproval } from "../../auto-approval"
 import { MessageQueueService } from "../../message-queue/MessageQueueService"
 import { Task } from "../Task"
+import { setAbortLatch } from "../../../test-utils/run-state"
 
 vi.mock("../../auto-approval", async (importOriginal) => {
 	const actual = await importOriginal<typeof import("../../auto-approval")>()
@@ -11,7 +12,7 @@ vi.mock("../../auto-approval", async (importOriginal) => {
 
 function buildTask(getState: () => Promise<Record<string, unknown>>) {
 	const task = Object.create(Task.prototype) as Task
-	task["abort"] = false
+	setAbortLatch(task, false)
 	task["clineMessages"] = []
 	task["lastMessageTs"] = undefined
 	task["addToClineMessages"] = vi.fn(async () => {})
@@ -44,7 +45,7 @@ describe("Task.ask abort during the auto-approval await", () => {
 		})
 
 		const askPromise = task.ask("tool", JSON.stringify({ tool: "readFile" }), partial)
-		task["abort"] = true
+		setAbortLatch(task, true)
 		gate.release()
 
 		await expect(askPromise).rejects.toThrow(/aborted/)
@@ -62,7 +63,7 @@ describe("Task.ask abort during the auto-approval await", () => {
 
 		const askPromise = task.ask("tool", JSON.stringify({ tool: "readFile" }), false)
 		await vi.waitFor(() => expect(checkAutoApproval).toHaveBeenCalled())
-		task["abort"] = true
+		setAbortLatch(task, true)
 		gate.release()
 
 		await expect(askPromise).rejects.toThrow(/aborted/)
@@ -83,7 +84,7 @@ describe("Task.ask abort during the auto-approval await", () => {
 		task["clineMessages"] = [{ ...original }]
 
 		const askPromise = task.ask("followup", "new", partial)
-		task["abort"] = true
+		setAbortLatch(task, true)
 		gate.release()
 
 		await expect(askPromise).rejects.toThrow(/aborted/)
@@ -102,7 +103,7 @@ describe("Task.ask abort during the auto-approval await", () => {
 		task.messageQueueService.addMessage("queued feedback")
 
 		const askPromise = task.ask("followup", "Q?", false)
-		task["abort"] = true
+		setAbortLatch(task, true)
 		gate.release()
 
 		await expect(askPromise).rejects.toThrow(/aborted/)

src/core/task/__tests__/ask-allowlist-cwd.spec.ts

@@ -3,6 +3,7 @@
 import type { ExtensionState } from "@roo-code/types"
 
 import { Task } from "../Task"
+import { setAbortLatch } from "../../../test-utils/run-state"
 
 // The allowlist patterns are resolved against a workspace root, and the path in
 // the tool message was made relative by `getReadablePath(task.cwd, ...)`. So the
@@ -23,7 +24,7 @@ type ProviderStub = {
 
 function buildTask(provider: ProviderStub, taskCwd: string) {
 	const task = Object.create(Task.prototype) as Task
-	task["abort"] = false
+	setAbortLatch(task, false)
 	task["clineMessages"] = []
 	task["askResponse"] = undefined
 	task["askResponseText"] = undefined

src/core/task/__tests__/ask-auto-deny.spec.ts

@@ -5,6 +5,7 @@ import { type ClineMessage, type ExtensionState, RooCodeEventName } from "@roo-c
 import * as autoApprovalModule from "../../auto-approval"
 import { createRateLimitClock } from "../RateLimitClock"
 import { Task } from "../Task"
+import { setAbortLatch } from "../../../test-utils/run-state"
 
 // The streaming-loop drive below never asserts on environment details; the
 // real collector reaches into the VS Code window API, which this file's
@@ -31,7 +32,7 @@ type ProviderStub = {
 
 function buildTask(provider: ProviderStub, taskCwd: string) {
 	const task = Object.create(Task.prototype) as Task
-	task["abort"] = false
+	setAbortLatch(task, false)
 	task["clineMessages"] = []
 	task["askResponse"] = undefined
 	task["askResponseText"] = undefined
@@ -743,7 +744,7 @@ describe("Task.ask queue path cannot bypass blanket deny", () => {
 		await vi.waitFor(() => expect(getStateCalls).toBe(2))
 		expect(queue.hasUnclaimed()).toBe(false)
 
-		task["abort"] = true
+		setAbortLatch(task, true)
 
 		// `ask()` must reject on abort; racing a deadline distinguishes a wrong
 		// settle from a hang on the pending read.
@@ -830,7 +831,7 @@ describe("Task.ask queue path cannot bypass blanket deny", () => {
 		// longer this wait runs, the less event-loop-stall slack is left before
 		// the 2 s timer could legitimately fire while the task is still live.
 		await vi.waitFor(() => expect(Date.now() - armedAt).toBeGreaterThanOrEqual(300), { timeout: 2_000 })
-		task["abort"] = true
+		setAbortLatch(task, true)
 
 		const outcome = await toolAsk.then(
 			() => "resolved",
@@ -931,7 +932,7 @@ describe("Task.ask queue path cannot bypass blanket deny", () => {
 		await vi.waitFor(() => expect(getStateCalls).toBe(2))
 		expect(queue.hasUnclaimed()).toBe(false)
 
-		task["abort"] = true
+		setAbortLatch(task, true)
 
 		// `ask()` must reject on abort; racing a deadline distinguishes a wrong
 		// settle from a hang on the pending read.
@@ -993,7 +994,7 @@ describe("Task.ask queue path cannot bypass blanket deny", () => {
 		await vi.waitFor(() => expect(getStateCalls).toBe(2))
 		expect(queue.hasUnclaimed()).toBe(false)
 
-		task["abort"] = true
+		setAbortLatch(task, true)
 
 		// `ask()` must reject on abort; racing a deadline distinguishes a wrong
 		// settle from a hang on the pending read.

src/core/task/__tests__/ask-clear-approval-buttons.spec.ts

@@ -1,4 +1,5 @@
 import { Task } from "../Task"
+import { setAbortLatch } from "../../../test-utils/run-state"
 
 // When the backend auto-resolves an interactive ask, isAnswered:true is stamped
 // on the ClineMessage before it is added so the webview state snapshot already
@@ -12,7 +13,7 @@ type ProviderStub = {
 
 function buildTask(provider: ProviderStub | undefined) {
 	const task = Object.create(Task.prototype) as Task
-	;(task as any).abort = false
+	setAbortLatch(task, false)
 	;(task as any).clineMessages = []
 	;(task as any).askResponse = undefined
 	;(task as any).askResponseText = undefined

src/core/task/__tests__/ask-queued-message-drain.spec.ts

@@ -1,11 +1,11 @@
 import { Task } from "../Task"
+import { setAbortLatch } from "../../../test-utils/run-state"
 
 type QueueTaskTestAccess = {
 	say: Task["say"]
 	saveClineMessages: () => Promise<boolean>
 	addToClineMessages: () => Promise<void>
 	lastMessageTs?: number
-	abort: boolean
 }
 
 const getQueueTaskTestAccess = (task: Task) => task as unknown as QueueTaskTestAccess
@@ -16,7 +16,7 @@ const getQueueTaskTestAccess = (task: Task) => task as unknown as QueueTaskTestA
 describe("Task.ask queued message drain", () => {
 	function createTask(provider?: { getState: () => Promise<Record<string, boolean>> }) {
 		const task = Object.create(Task.prototype) as Task
-		;(task as any).abort = false
+		setAbortLatch(task, false)
 		;(task as any).clineMessages = []
 		;(task as any).askResponse = undefined
 		;(task as any).askResponseText = undefined
@@ -346,7 +346,7 @@ describe("Task.ask queued message drain", () => {
 		task.messageQueueService.addMessage("Persist me later")
 		const ask = task.ask("completion_result", "Done", false)
 		await Promise.resolve()
-		access.abort = true
+		setAbortLatch(task, true)
 		finishAddingAsk()
 
 		await expect(ask).rejects.toThrow("aborted")
@@ -398,7 +398,7 @@ describe("Task.ask queued message drain", () => {
 			await vi.advanceTimersByTimeAsync(0)
 			expect(access.saveClineMessages).toHaveBeenCalledTimes(1)
 
-			access.abort = true
+			setAbortLatch(task, true)
 			await vi.advanceTimersByTimeAsync(250)
 
 			await expect(persistence).resolves.toBe(false)

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

@@ -12,6 +12,7 @@ import { Task } from "../Task"
 import { ClineProvider } from "../../webview/ClineProvider"
 import { ContextProxy } from "../../config/ContextProxy"
 import { providerIdentifiers } from "@roo-code/types/provider-identifiers"
+import { setAbortLatch } from "../../../test-utils/run-state"
 
 // Mock delay before any imports that might use it
 vi.mock("delay", () => ({
@@ -458,7 +459,7 @@ describe("flushPendingToolResultsToHistory", () => {
 
 		const waitForPersistence = vi.spyOn(task, "waitForCurrentAssistantMessagePersistence")
 
-		task.abort = true
+		setAbortLatch(task, true)
 
 		await task.flushPendingToolResultsToHistory()