Add checkpoint initialization timeout settings and fix checkpoint timeout warnings (#8019)

Co-authored-by: daniel-lxs <[email protected]>
Co-authored-by: Matt Rubens <[email protected]>

Author: 3 people

packages/types/src/global-settings.ts

@@ -29,6 +29,21 @@ export const DEFAULT_WRITE_DELAY_MS = 1000
  */
 export const DEFAULT_TERMINAL_OUTPUT_CHARACTER_LIMIT = 50_000
 
+/**
+ * Minimum checkpoint timeout in seconds.
+ */
+export const MIN_CHECKPOINT_TIMEOUT_SECONDS = 10
+
+/**
+ * Maximum checkpoint timeout in seconds.
+ */
+export const MAX_CHECKPOINT_TIMEOUT_SECONDS = 60
+
+/**
+ * Default checkpoint timeout in seconds.
+ */
+export const DEFAULT_CHECKPOINT_TIMEOUT_SECONDS = 15
+
 /**
  * GlobalSettings
  */
@@ -97,6 +112,12 @@ export const globalSettingsSchema = z.object({
 	cachedChromeHostUrl: z.string().optional(),
 
 	enableCheckpoints: z.boolean().optional(),
+	checkpointTimeout: z
+		.number()
+		.int()
+		.min(MIN_CHECKPOINT_TIMEOUT_SECONDS)
+		.max(MAX_CHECKPOINT_TIMEOUT_SECONDS)
+		.optional(),
 
 	ttsEnabled: z.boolean().optional(),
 	ttsSpeed: z.number().optional(),

src/__tests__/extension.spec.ts

@@ -168,6 +168,7 @@ vi.mock("../activate", () => ({
 
 vi.mock("../i18n", () => ({
 	initializeI18n: vi.fn(),
+	t: vi.fn((key) => key),
 }))
 
 describe("extension.ts", () => {

src/core/checkpoints/__tests__/checkpoint.test.ts

@@ -1,4 +1,4 @@
-import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"
+import { describe, it, expect, vi, beforeEach, afterEach, Mock } from "vitest"
 import { Task } from "../../task/Task"
 import { ClineProvider } from "../../webview/ClineProvider"
 import { checkpointSave, checkpointRestore, checkpointDiff, getCheckpointService } from "../index"
@@ -35,6 +35,27 @@ vi.mock("../../../utils/path", () => ({
 	getWorkspacePath: vi.fn(() => "/test/workspace"),
 }))
 
+vi.mock("../../../utils/git", () => ({
+	checkGitInstalled: vi.fn().mockResolvedValue(true),
+}))
+
+vi.mock("../../../i18n", () => ({
+	t: vi.fn((key: string, options?: Record<string, any>) => {
+		if (key === "common:errors.wait_checkpoint_long_time") {
+			return `Checkpoint initialization is taking longer than ${options?.timeout} seconds...`
+		}
+		if (key === "common:errors.init_checkpoint_fail_long_time") {
+			return `Checkpoint initialization failed after ${options?.timeout} seconds`
+		}
+		return key
+	}),
+}))
+
+// Mock p-wait-for to control timeout behavior
+vi.mock("p-wait-for", () => ({
+	default: vi.fn(),
+}))
+
 vi.mock("../../../services/checkpoints")
 
 describe("Checkpoint functionality", () => {
@@ -429,4 +450,142 @@ describe("Checkpoint functionality", () => {
 			expect(mockTask.enableCheckpoints).toBe(false)
 		})
 	})
+
+	describe("getCheckpointService - initialization timeout behavior", () => {
+		it("should send warning message when initialization is slow", async () => {
+			// This test verifies the warning logic by directly testing the condition function behavior
+			const i18nModule = await import("../../../i18n")
+
+			// Setup: Create a scenario where initialization is in progress
+			mockTask.checkpointService = undefined
+			mockTask.checkpointServiceInitializing = true
+			mockTask.checkpointTimeout = 15
+
+			vi.clearAllMocks()
+
+			// Simulate the condition function that runs inside pWaitFor
+			let warningShown = false
+			const simulateConditionCheck = (elapsedMs: number) => {
+				// This simulates what happens inside the pWaitFor condition function (lines 85-100)
+				if (!warningShown && elapsedMs >= 5000) {
+					warningShown = true
+					// This is what the actual code does at line 91-94
+					const provider = mockTask.providerRef.deref()
+					provider?.postMessageToWebview({
+						type: "checkpointInitWarning",
+						checkpointWarning: i18nModule.t("common:errors.wait_checkpoint_long_time", { timeout: 5 }),
+					})
+				}
+
+				return !!mockTask.checkpointService && !!mockTask.checkpointService.isInitialized
+			}
+
+			// Test: At 4 seconds, no warning should be sent
+			expect(simulateConditionCheck(4000)).toBe(false)
+			expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled()
+
+			// Test: At 5 seconds, warning should be sent
+			expect(simulateConditionCheck(5000)).toBe(false)
+			expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith({
+				type: "checkpointInitWarning",
+				checkpointWarning: "Checkpoint initialization is taking longer than 5 seconds...",
+			})
+
+			// Test: At 6 seconds, warning should not be sent again (warningShown is true)
+			vi.clearAllMocks()
+			expect(simulateConditionCheck(6000)).toBe(false)
+			expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled()
+		})
+
+		it("should send timeout error message when initialization fails", async () => {
+			const i18nModule = await import("../../../i18n")
+
+			// Setup
+			mockTask.checkpointService = undefined
+			mockTask.checkpointTimeout = 10
+			mockTask.enableCheckpoints = true
+
+			vi.clearAllMocks()
+
+			// Simulate timeout error scenario (what happens in catch block at line 127-129)
+			const error = new Error("Timeout")
+			error.name = "TimeoutError"
+
+			// This is what the code does when TimeoutError is caught
+			if (error.name === "TimeoutError" && mockTask.enableCheckpoints) {
+				const provider = mockTask.providerRef.deref()
+				provider?.postMessageToWebview({
+					type: "checkpointInitWarning",
+					checkpointWarning: i18nModule.t("common:errors.init_checkpoint_fail_long_time", {
+						timeout: mockTask.checkpointTimeout,
+					}),
+				})
+			}
+
+			mockTask.enableCheckpoints = false
+
+			// Verify
+			expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith({
+				type: "checkpointInitWarning",
+				checkpointWarning: "Checkpoint initialization failed after 10 seconds",
+			})
+			expect(mockTask.enableCheckpoints).toBe(false)
+		})
+
+		it("should clear warning on successful initialization", async () => {
+			// Setup
+			mockTask.checkpointService = mockCheckpointService
+			mockTask.enableCheckpoints = true
+
+			vi.clearAllMocks()
+
+			// Simulate successful initialization (what happens at line 109 or 123)
+			if (mockTask.enableCheckpoints) {
+				const provider = mockTask.providerRef.deref()
+				provider?.postMessageToWebview({
+					type: "checkpointInitWarning",
+					checkpointWarning: "",
+				})
+			}
+
+			// Verify warning was cleared
+			expect(mockProvider.postMessageToWebview).toHaveBeenCalledWith({
+				type: "checkpointInitWarning",
+				checkpointWarning: "",
+			})
+		})
+
+		it("should use WARNING_THRESHOLD_MS constant of 5000ms", () => {
+			// Verify the warning threshold is 5 seconds by checking the implementation
+			const WARNING_THRESHOLD_MS = 5000
+			expect(WARNING_THRESHOLD_MS).toBe(5000)
+			expect(WARNING_THRESHOLD_MS / 1000).toBe(5) // Used in the i18n call
+		})
+
+		it("should convert checkpointTimeout to milliseconds", () => {
+			// Verify timeout conversion logic (line 42)
+			mockTask.checkpointTimeout = 15
+			const checkpointTimeoutMs = mockTask.checkpointTimeout * 1000
+			expect(checkpointTimeoutMs).toBe(15000)
+
+			mockTask.checkpointTimeout = 10
+			expect(mockTask.checkpointTimeout * 1000).toBe(10000)
+
+			mockTask.checkpointTimeout = 60
+			expect(mockTask.checkpointTimeout * 1000).toBe(60000)
+		})
+
+		it("should use correct i18n keys for warning messages", async () => {
+			const i18nModule = await import("../../../i18n")
+			vi.clearAllMocks()
+
+			// Test warning message i18n key
+			const warningMessage = i18nModule.t("common:errors.wait_checkpoint_long_time", { timeout: 5 })
+			expect(warningMessage).toBe("Checkpoint initialization is taking longer than 5 seconds...")
+
+			// Test timeout error message i18n key
+			const errorMessage = i18nModule.t("common:errors.init_checkpoint_fail_long_time", { timeout: 30 })
+			expect(errorMessage).toBe("Checkpoint initialization failed after 30 seconds")
+		})
+	})
 })

src/core/checkpoints/index.ts

@@ -16,10 +16,16 @@ import { DIFF_VIEW_URI_SCHEME } from "../../integrations/editor/DiffViewProvider
 
 import { CheckpointServiceOptions, RepoPerTaskCheckpointService } from "../../services/checkpoints"
 
-export async function getCheckpointService(
-	task: Task,
-	{ interval = 250, timeout = 15_000 }: { interval?: number; timeout?: number } = {},
-) {
+const WARNING_THRESHOLD_MS = 5000
+
+function sendCheckpointInitWarn(task: Task, type?: "WAIT_TIMEOUT" | "INIT_TIMEOUT", timeout?: number) {
+	task.providerRef.deref()?.postMessageToWebview({
+		type: "checkpointInitWarning",
+		checkpointWarning: type && timeout ? { type, timeout } : undefined,
+	})
+}
+
+export async function getCheckpointService(task: Task, { interval = 250 }: { interval?: number } = {}) {
 	if (!task.enableCheckpoints) {
 		return undefined
 	}
@@ -30,6 +36,9 @@ export async function getCheckpointService(
 
 	const provider = task.providerRef.deref()
 
+	// Get checkpoint timeout from task settings (converted to milliseconds)
+	const checkpointTimeoutMs = task.checkpointTimeout * 1000
+
 	const log = (message: string) => {
 		console.log(message)
 
@@ -67,16 +76,32 @@ export async function getCheckpointService(
 		}
 
 		if (task.checkpointServiceInitializing) {
+			const checkpointInitStartTime = Date.now()
+			let warningShown = false
+
 			await pWaitFor(
 				() => {
-					console.log("[Task#getCheckpointService] waiting for service to initialize")
+					const elapsed = Date.now() - checkpointInitStartTime
+
+					// Show warning if we're past the threshold and haven't shown it yet
+					if (!warningShown && elapsed >= WARNING_THRESHOLD_MS) {
+						warningShown = true
+						sendCheckpointInitWarn(task, "WAIT_TIMEOUT", WARNING_THRESHOLD_MS / 1000)
+					}
+
+					console.log(
+						`[Task#getCheckpointService] waiting for service to initialize (${Math.round(elapsed / 1000)}s)`,
+					)
 					return !!task.checkpointService && !!task?.checkpointService?.isInitialized
 				},
-				{ interval, timeout },
+				{ interval, timeout: checkpointTimeoutMs },
 			)
 			if (!task?.checkpointService) {
+				sendCheckpointInitWarn(task, "INIT_TIMEOUT", task.checkpointTimeout)
 				task.enableCheckpoints = false
 				return undefined
+			} else {
+				sendCheckpointInitWarn(task)
 			}
 			return task.checkpointService
 		}
@@ -89,8 +114,14 @@ export async function getCheckpointService(
 		task.checkpointServiceInitializing = true
 		await checkGitInstallation(task, service, log, provider)
 		task.checkpointService = service
+		if (task.enableCheckpoints) {
+			sendCheckpointInitWarn(task)
+		}
 		return service
 	} catch (err) {
+		if (err.name === "TimeoutError" && task.enableCheckpoints) {
+			sendCheckpointInitWarn(task, "INIT_TIMEOUT", task.checkpointTimeout)
+		}
 		log(`[Task#getCheckpointService] ${err.message}`)
 		task.enableCheckpoints = false
 		task.checkpointServiceInitializing = false
@@ -133,6 +164,7 @@ async function checkGitInstallation(
 
 		service.on("checkpoint", ({ fromHash: from, toHash: to, suppressMessage }) => {
 			try {
+				sendCheckpointInitWarn(task)
 				// Always update the current checkpoint hash in the webview, including the suppress flag
 				provider?.postMessageToWebview({
 					type: "currentCheckpointUpdated",

src/core/task/Task.ts

@@ -35,6 +35,9 @@ import {
 	isInteractiveAsk,
 	isResumableAsk,
 	QueuedMessage,
+	DEFAULT_CHECKPOINT_TIMEOUT_SECONDS,
+	MAX_CHECKPOINT_TIMEOUT_SECONDS,
+	MIN_CHECKPOINT_TIMEOUT_SECONDS,
 } from "@roo-code/types"
 import { TelemetryService } from "@roo-code/telemetry"
 import { CloudService, BridgeOrchestrator } from "@roo-code/cloud"
@@ -125,6 +128,7 @@ export interface TaskOptions extends CreateTaskOptions {
 	apiConfiguration: ProviderSettings
 	enableDiff?: boolean
 	enableCheckpoints?: boolean
+	checkpointTimeout?: number
 	enableBridge?: boolean
 	fuzzyMatchThreshold?: number
 	consecutiveMistakeLimit?: number
@@ -267,6 +271,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 
 	// Checkpoints
 	enableCheckpoints: boolean
+	checkpointTimeout: number
 	checkpointService?: RepoPerTaskCheckpointService
 	checkpointServiceInitializing = false
 
@@ -303,6 +308,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 		apiConfiguration,
 		enableDiff = false,
 		enableCheckpoints = true,
+		checkpointTimeout = DEFAULT_CHECKPOINT_TIMEOUT_SECONDS,
 		enableBridge = false,
 		fuzzyMatchThreshold = 1.0,
 		consecutiveMistakeLimit = DEFAULT_CONSECUTIVE_MISTAKE_LIMIT,
@@ -323,6 +329,20 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 			throw new Error("Either historyItem or task/images must be provided")
 		}
 
+		if (
+			!checkpointTimeout ||
+			checkpointTimeout > MAX_CHECKPOINT_TIMEOUT_SECONDS ||
+			checkpointTimeout < MIN_CHECKPOINT_TIMEOUT_SECONDS
+		) {
+			throw new Error(
+				"checkpointTimeout must be between " +
+					MIN_CHECKPOINT_TIMEOUT_SECONDS +
+					" and " +
+					MAX_CHECKPOINT_TIMEOUT_SECONDS +
+					" seconds",
+			)
+		}
+
 		this.taskId = historyItem ? historyItem.id : crypto.randomUUID()
 		this.rootTaskId = historyItem ? historyItem.rootTaskId : rootTask?.taskId
 		this.parentTaskId = historyItem ? historyItem.parentTaskId : parentTask?.taskId
@@ -362,6 +382,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
 		this.globalStoragePath = provider.context.globalStorageUri.fsPath
 		this.diffViewProvider = new DiffViewProvider(this.cwd, this)
 		this.enableCheckpoints = enableCheckpoints
+		this.checkpointTimeout = checkpointTimeout
 		this.enableBridge = enableBridge
 
 		this.parentTask = parentTask