[Fix] Restart-persistence E2E flakes when the final ui_messages.json write lands after VS Code quits

apps/vscode-e2e/src/suite/restart-persistence.test.ts

@@ -54,6 +54,13 @@ async function runCreate(api: RooCodeAPI): Promise<void> {
 		await waitUntilCompleted({ api, taskId })
 		assert.strictEqual(sawMarker, true, `Completion should include ${MARKER}`)
 
+		// TaskCompleted fires before the final saveClineMessages() write is
+		// necessarily durable on disk. Wait until the completion marker is
+		// persisted in ui_messages.json before quitting, so the verify phase
+		// never reads a task directory whose final write has not landed yet.
+		const createdTaskId: string = taskId
+		await waitFor(() => api.hasTaskUiMessageText(createdTaskId, MARKER))
+
 		const result: PhaseResult = {
 			version: PHASE_RESULT_VERSION,
 			phase: "create",

packages/types/src/api.ts

@@ -68,6 +68,15 @@ export interface RooCodeAPI extends EventEmitter<RooCodeAPIEvents> {
 		taskId: string,
 		sequence: TaskApiConversationHistorySequence,
 	): Promise<boolean>
+	/**
+	 * Checks whether the persisted ui_messages.json for a task contains a
+	 * message with the given text. Reads from disk, so it only returns true
+	 * once the final write is durable. Intended for use in tests only.
+	 * @param taskId The ID of the task.
+	 * @param text The text to look for in persisted UI messages.
+	 * @returns True when a persisted UI message contains the text, or false if unavailable.
+	 */
+	hasTaskUiMessageText(taskId: string, text: string): Promise<boolean>
 	/**
 	 * Returns the current task stack.
 	 * @returns An array of task IDs.

src/core/task-persistence/__tests__/taskMessages.spec.ts

@@ -20,6 +20,7 @@ vi.mock("../../../utils/safeWriteJson", () => ({
 
 // Import after mocks
 import { saveTaskMessages, readTaskMessages } from "../taskMessages"
+import { formatErrorDetail } from "../readFileWithMissingRetry"
 
 let tmpBaseDir: string
 
@@ -99,6 +100,23 @@ describe("taskMessages.saveTaskMessages", () => {
 			expect.objectContaining({ ts: 2, text: "incoming" }),
 		])
 	})
+
+	it("propagates a forced write failure without swallowing or replacing it", async () => {
+		// Fault injection: simulate an interrupted/failed final write (e.g. the
+		// extension host quitting mid-save or the disk refusing the write). The
+		// save must reject with the original error so callers never treat the
+		// messages as durably persisted, and the underlying code stays visible.
+		const writeFailure = Object.assign(new Error("no space left on device"), { code: "ENOSPC" })
+		hoisted.safeWriteJsonMock.mockRejectedValueOnce(writeFailure)
+
+		await expect(
+			saveTaskMessages({
+				messages: [{ ts: 1, type: "say", say: "text", text: "unsaved" }],
+				taskId: "task-write-failure",
+				globalStoragePath: tmpBaseDir,
+			}),
+		).rejects.toBe(writeFailure)
+	})
 })
 
 describe("taskMessages.readTaskMessages", () => {
@@ -151,24 +169,63 @@ describe("taskMessages.readTaskMessages", () => {
 		expect(hoisted.readFileMock).toHaveBeenCalledTimes(2)
 	})
 
-	it("throws when the missing-file retry also fails", async () => {
+	it("throws with the underlying error code when the missing-file retry also fails", async () => {
 		vi.spyOn(Math, "random").mockReturnValue(0)
 		const missing = Object.assign(new Error("missing"), { code: "ENOENT" })
 		hoisted.readFileMock.mockRejectedValue(missing)
 
-		await expect(
-			readTaskMessages({ taskId: "task-still-missing", globalStoragePath: tmpBaseDir }),
-		).rejects.toMatchObject({ kind: "not_found" })
+		const error = await readTaskMessages({ taskId: "task-still-missing", globalStoragePath: tmpBaseDir }).catch(
+			(caught: unknown) => caught,
+		)
+		expect(error).toMatchObject({
+			kind: "not_found",
+			originalError: missing,
+			message: expect.stringContaining("(code: ENOENT)"),
+		})
 		expect(hoisted.readFileMock).toHaveBeenCalledTimes(2)
 	})
 
-	it("does not retry non-ENOENT read failures", async () => {
+	it("does not retry non-ENOENT read failures and surfaces the underlying code", async () => {
 		const denied = Object.assign(new Error("denied"), { code: "EACCES" })
 		hoisted.readFileMock.mockRejectedValueOnce(denied)
 
-		await expect(readTaskMessages({ taskId: "task-denied", globalStoragePath: tmpBaseDir })).rejects.toMatchObject({
-			kind: "io_error",
-		})
+		const error = await readTaskMessages({ taskId: "task-denied", globalStoragePath: tmpBaseDir }).catch(
+			(caught: unknown) => caught,
+		)
+		expect(error).toMatchObject({ kind: "io_error" })
+		expect((error as Error).message).toContain("(code: EACCES)")
 		expect(hoisted.readFileMock).toHaveBeenCalledTimes(1)
 	})
+
+	it("surfaces the parse error detail for a torn (half-written) file", async () => {
+		// Fault injection: a truncated/torn write leaves invalid JSON on disk.
+		// The read error must identify the parse failure (not a missing file)
+		// so a torn write is distinguishable from a late write in the logs.
+		const taskId = "task-torn-write"
+		const taskDir = path.join(tmpBaseDir, "tasks", taskId)
+		await fs.mkdir(taskDir, { recursive: true })
+		await fs.writeFile(path.join(taskDir, "ui_messages.json"), '[{"ts":1,"type":"sa', "utf8")
+
+		const error = await readTaskMessages({ taskId, globalStoragePath: tmpBaseDir }).catch(
+			(caught: unknown) => caught,
+		)
+		expect(error).toMatchObject({ kind: "invalid" })
+		expect((error as Error).message).toContain("SyntaxError")
+	})
+})
+
+describe("readFileWithMissingRetry formatErrorDetail", () => {
+	it("prefers the filesystem error code when present", () => {
+		expect(formatErrorDetail(Object.assign(new Error("missing"), { code: "ENOENT" }))).toBe(" (code: ENOENT)")
+	})
+
+	it("falls back to the error name and message", () => {
+		expect(formatErrorDetail(new SyntaxError("Unexpected end of JSON input"))).toBe(
+			" (SyntaxError: Unexpected end of JSON input)",
+		)
+	})
+
+	it("stringifies non-error causes", () => {
+		expect(formatErrorDetail("boom")).toBe(" (boom)")
+	})
 })

src/core/task-persistence/readFileWithMissingRetry.ts

@@ -1,5 +1,8 @@
 import * as fs from "fs/promises"
 
+// safeWriteJson replaces a file in two renames (target to backup, new file to
+// target) because a single rename over an open file fails with EPERM on
+// Windows. A reader can see ENOENT between the two renames, so retry once.
 const READ_RETRY_MIN_MS = 1
 const READ_RETRY_MAX_MS = 10
 const READ_RETRY_RANGE_MS = READ_RETRY_MAX_MS - READ_RETRY_MIN_MS + 1
@@ -10,6 +13,24 @@ export function getErrorCode(error: unknown): string | undefined {
 		: undefined
 }
 
+/**
+ * Formats the actionable detail of an underlying error for inclusion in
+ * wrapping error messages: the filesystem error code when present (e.g.
+ * ENOENT vs EACCES), otherwise the error name and message (e.g. SyntaxError
+ * for a torn/half-written JSON file). Keeps wrappers such as
+ * TaskMessagesReadError diagnosable without logging the full cause chain.
+ */
+export function formatErrorDetail(error: unknown): string {
+	const code = getErrorCode(error)
+	if (code) {
+		return ` (code: ${code})`
+	}
+	if (error instanceof Error) {
+		return ` (${error.name}: ${error.message})`
+	}
+	return ` (${String(error)})`
+}
+
 export async function readFileWithMissingRetry(filePath: string): Promise<string> {
 	try {
 		return await fs.readFile(filePath, "utf8")

src/core/task-persistence/taskMessages.ts

@@ -6,7 +6,7 @@ import { safeWriteJson } from "../../utils/safeWriteJson"
 import { GlobalFileNames } from "../../shared/globalFileNames"
 import { getTaskDirectoryPath } from "../../utils/storage"
 import { ensureMessageIdentifiers, mergeClineMessageSnapshots } from "./mergeMessageSnapshots"
-import { getErrorCode, readFileWithMissingRetry } from "./readFileWithMissingRetry"
+import { formatErrorDetail, getErrorCode, readFileWithMissingRetry } from "./readFileWithMissingRetry"
 
 export type TaskMessagesReadErrorKind = "not_found" | "invalid" | "io_error"
 
@@ -38,14 +38,22 @@ export async function readTaskMessages({
 		fileContent = await readFileWithMissingRetry(filePath)
 	} catch (error) {
 		const kind = getErrorCode(error) === "ENOENT" ? "not_found" : "io_error"
-		throw new TaskMessagesReadError(kind, `Failed to read task messages for ${taskId} at ${filePath}`, error)
+		throw new TaskMessagesReadError(
+			kind,
+			`Failed to read task messages for ${taskId} at ${filePath}${formatErrorDetail(error)}`,
+			error,
+		)
 	}
 
 	let parsedData: unknown
 	try {
 		parsedData = JSON.parse(fileContent)
 	} catch (error) {
-		throw new TaskMessagesReadError("invalid", `Failed to parse task messages for ${taskId} at ${filePath}`, error)
+		throw new TaskMessagesReadError(
+			"invalid",
+			`Failed to parse task messages for ${taskId} at ${filePath}${formatErrorDetail(error)}`,
+			error,
+		)
 	}
 
 	if (!Array.isArray(parsedData)) {

src/extension/__tests__/api-has-task-ui-message-text.spec.ts

@@ -0,0 +1,88 @@
+import { describe, it, expect, vi, beforeEach } from "vitest"
+import * as vscode from "vscode"
+
+import { API } from "../api"
+import { ClineProvider } from "../../core/webview/ClineProvider"
+import { readTaskMessages, TaskMessagesReadError } from "../../core/task-persistence/taskMessages"
+import { makeClineProviderFactory } from "../../test-utils/provider"
+
+vi.mock("vscode")
+vi.mock("../../core/webview/ClineProvider")
+vi.mock("../../core/task-persistence/taskMessages", async (importOriginal) => ({
+	...(await importOriginal<typeof import("../../core/task-persistence/taskMessages")>()),
+	readTaskMessages: vi.fn(),
+}))
+
+describe("API#hasTaskUiMessageText", () => {
+	let api: API
+	let mockProvider: ClineProvider
+
+	beforeEach(() => {
+		vi.mocked(readTaskMessages).mockReset()
+
+		// API logging only needs appendLine in this suite; a full OutputChannel fake would obscure the tested contract.
+		const mockOutputChannel = {
+			appendLine: vi.fn(),
+		} as unknown as vscode.OutputChannel
+
+		// The method under test only reads contextProxy.globalStorageUri; the
+		// remaining members exist so the API constructor's listener wiring works.
+		mockProvider = {
+			context: {} as vscode.ExtensionContext,
+			contextProxy: { globalStorageUri: { fsPath: "/mock/global/storage" } },
+			taskHistoryStore: { get: vi.fn() },
+			getTaskWithId: vi.fn().mockResolvedValue({}),
+			on: vi.fn(),
+		} as unknown as ClineProvider
+
+		api = new API(mockOutputChannel, mockProvider, makeClineProviderFactory())
+	})
+
+	it("returns true when a persisted ui message contains the text", async () => {
+		vi.mocked(readTaskMessages).mockResolvedValue([
+			{ ts: 1, type: "say", say: "text", text: "done RESTART_PERSISTENCE_MARKER" },
+		])
+
+		await expect(api.hasTaskUiMessageText("task-1", "RESTART_PERSISTENCE_MARKER")).resolves.toBe(true)
+		expect(readTaskMessages).toHaveBeenCalledWith({ taskId: "task-1", globalStoragePath: "/mock/global/storage" })
+	})
+
+	it("returns false when no persisted ui message contains the text", async () => {
+		vi.mocked(readTaskMessages).mockResolvedValue([{ ts: 1, type: "say", say: "text", text: "unrelated" }])
+
+		await expect(api.hasTaskUiMessageText("task-1", "RESTART_PERSISTENCE_MARKER")).resolves.toBe(false)
+	})
+
+	it("finds the text in a later message and skips messages without text", async () => {
+		vi.mocked(readTaskMessages).mockResolvedValue([
+			{ ts: 1, type: "say", say: "api_req_started" },
+			{ ts: 2, type: "say", say: "text", text: "unrelated" },
+			{ ts: 3, type: "say", say: "completion_result", text: "done RESTART_PERSISTENCE_MARKER" },
+		])
+
+		await expect(api.hasTaskUiMessageText("task-1", "RESTART_PERSISTENCE_MARKER")).resolves.toBe(true)
+	})
+
+	it("returns false when only messages without text exist", async () => {
+		vi.mocked(readTaskMessages).mockResolvedValue([{ ts: 1, type: "say", say: "api_req_started" }])
+
+		await expect(api.hasTaskUiMessageText("task-1", "RESTART_PERSISTENCE_MARKER")).resolves.toBe(false)
+	})
+
+	it.each(["not_found", "invalid", "io_error"] as const)(
+		"returns false instead of throwing on a %s read failure",
+		async (kind) => {
+			// The completion write may not have landed yet; callers poll until it does.
+			vi.mocked(readTaskMessages).mockRejectedValue(new TaskMessagesReadError(kind, `read failed: ${kind}`))
+
+			await expect(api.hasTaskUiMessageText("task-1", "RESTART_PERSISTENCE_MARKER")).resolves.toBe(false)
+		},
+	)
+
+	it("returns false without reading disk for a task id that is not in history", async () => {
+		vi.mocked(mockProvider.getTaskWithId).mockRejectedValue(new Error("Task not found"))
+
+		await expect(api.hasTaskUiMessageText("../../escape", "RESTART_PERSISTENCE_MARKER")).resolves.toBe(false)
+		expect(readTaskMessages).not.toHaveBeenCalled()
+	})
+})

src/extension/api.ts

@@ -28,6 +28,7 @@ import { Package } from "../shared/package"
 import type { Mode } from "../shared/modes"
 import type { ClineProvider } from "../core/webview/ClineProvider"
 import type { ClineProviderFactory } from "../core/webview/ClineProviderFactory"
+import { readTaskMessages } from "../core/task-persistence/taskMessages"
 import { Terminal } from "../integrations/terminal/Terminal"
 import { TerminalRegistry } from "../integrations/terminal/TerminalRegistry"
 import { getCommands } from "../services/command/commands"
@@ -305,6 +306,22 @@ export class API extends EventEmitter<RooCodeEvents> implements RooCodeAPI {
 		)
 	}
 
+	/** Checks persisted ui_messages.json contents without exposing messages to tests. */
+	public async hasTaskUiMessageText(taskId: string, text: string): Promise<boolean> {
+		let uiMessages: Awaited<ReturnType<typeof readTaskMessages>>
+		try {
+			// Reject unknown ids first: readTaskMessages creates the task directory.
+			await this.sidebarProvider.getTaskWithId(taskId)
+			uiMessages = await readTaskMessages({
+				taskId,
+				globalStoragePath: this.sidebarProvider.contextProxy.globalStorageUri.fsPath,
+			})
+		} catch {
+			return false
+		}
+		return uiMessages.some((message) => message.text?.includes(text))
+	}
+
 	public getCurrentTaskStack() {
 		return this.sidebarProvider.getCurrentTaskStack()
 	}