From e619a6592f79670709d767a25187cdce33c475f9 Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Fri, 30 Oct 2026 13:44:45 -0700 Subject: [PATCH 01/15] fix: validate checkpoint before restoring workspace --- .../checkpoints/ShadowCheckpointService.ts | 17 +++++++ .../__tests__/ShadowCheckpointService.spec.ts | 44 +++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/src/services/checkpoints/ShadowCheckpointService.ts b/src/services/checkpoints/ShadowCheckpointService.ts index b420d40e35..9f01e5257f 100644 --- a/src/services/checkpoints/ShadowCheckpointService.ts +++ b/src/services/checkpoints/ShadowCheckpointService.ts @@ -377,6 +377,17 @@ export abstract class ShadowCheckpointService extends EventEmitter { } } + // MUST run before workspace-mutating Git commands: discovering a missing commit + // after git clean would leave untracked user files permanently deleted. + private async commitExists(commitHash: string): Promise { + try { + await this.git!.raw(["cat-file", "-e", `${commitHash}^{commit}`]) + return true + } catch { + return false + } + } + public async restoreCheckpoint(commitHash: string) { try { this.log(`[${this.constructor.name}#restoreCheckpoint] starting checkpoint restore`) @@ -385,6 +396,12 @@ export abstract class ShadowCheckpointService extends EventEmitter { throw new Error("Shadow git repo not initialized") } + if (!(await this.commitExists(commitHash))) { + throw new Error( + `Cannot restore checkpoint ${commitHash}: this checkpoint no longer exists in the checkpoint repository. No files were changed.`, + ) + } + const start = Date.now() await this.git.clean("f", ["-d", "-f"]) await this.git.reset(["--hard", commitHash]) diff --git a/src/services/checkpoints/__tests__/ShadowCheckpointService.spec.ts b/src/services/checkpoints/__tests__/ShadowCheckpointService.spec.ts index 76e98367a5..27f7d05ecb 100644 --- a/src/services/checkpoints/__tests__/ShadowCheckpointService.spec.ts +++ b/src/services/checkpoints/__tests__/ShadowCheckpointService.spec.ts @@ -320,6 +320,50 @@ describe.each([[RepoPerTaskCheckpointService, "RepoPerTaskCheckpointService"]])( await expect(fs.readFile(untrackedFile, "utf-8")).rejects.toThrow() }) + it("does not modify the workspace when restoring a missing commit", async () => { + await fs.writeFile(testFile, "Committed content") + const commit = await service.saveCheckpoint("Checkpoint before loss") + expect(commit?.commit).toBeTruthy() + + const untrackedFile = path.join(service.workspaceDir, "precious-untracked.txt") + await fs.writeFile(untrackedFile, "Unsaved user work") + await fs.writeFile(testFile, "Uncommitted edit") + + const errorHandler = vi.fn() + const restoreHandler = vi.fn() + service.on("error", errorHandler) + service.on("restore", restoreHandler) + const checkpointsBeforeRestore = service.getCheckpoints() + const missingCommitHash = "0".repeat(40) + const expectedMessage = `Cannot restore checkpoint ${missingCommitHash}: this checkpoint no longer exists in the checkpoint repository. No files were changed.` + + await expect(service.restoreCheckpoint(missingCommitHash)).rejects.toThrow(expectedMessage) + + expect(await fs.readFile(untrackedFile, "utf-8")).toBe("Unsaved user work") + expect(await fs.readFile(testFile, "utf-8")).toBe("Uncommitted edit") + expect(service.getCheckpoints()).toEqual(checkpointsBeforeRestore) + expect(errorHandler).toHaveBeenCalledExactlyOnceWith({ + type: "error", + error: new Error(expectedMessage), + }) + expect(restoreHandler).not.toHaveBeenCalled() + }) + + it("still restores normally to a valid commit", async () => { + await fs.writeFile(testFile, "Checkpointed content") + const commit = await service.saveCheckpoint("Valid checkpoint") + expect(commit?.commit).toBeTruthy() + + await fs.writeFile(testFile, "Later edit") + const laterTrackedFile = path.join(service.workspaceDir, "created-after.txt") + await fs.writeFile(laterTrackedFile, "Created after checkpoint") + await service.saveCheckpoint("After checkpoint") + + await service.restoreCheckpoint(commit!.commit) + expect(await fs.readFile(testFile, "utf-8")).toBe("Checkpointed content") + await expect(fs.readFile(laterTrackedFile, "utf-8")).rejects.toThrow() + }) + it("does not create a checkpoint for ignored files", async () => { // Create a file that matches an ignored pattern (e.g., .log file). const ignoredFile = path.join(service.workspaceDir, "ignored.log") From e7a5c57b0e4bcf6b7502259733b64fcc20975fa7 Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Fri, 30 Oct 2026 14:09:04 -0700 Subject: [PATCH 02/15] fix(checkpoints): only treat missing objects as missing checkpoints Rethrow unexpected git cat-file failures from commitExists so restore reports the real error instead of disabling checkpoints. Add tests for a saved checkpoint whose commit object was removed, for unexpected Git failures, and for cleanup of unsaved files on a valid restore. --- .../checkpoints/ShadowCheckpointService.ts | 9 +++- .../__tests__/ShadowCheckpointService.spec.ts | 44 +++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/src/services/checkpoints/ShadowCheckpointService.ts b/src/services/checkpoints/ShadowCheckpointService.ts index 9f01e5257f..3b00def9a0 100644 --- a/src/services/checkpoints/ShadowCheckpointService.ts +++ b/src/services/checkpoints/ShadowCheckpointService.ts @@ -379,12 +379,17 @@ export abstract class ShadowCheckpointService extends EventEmitter { // MUST run before workspace-mutating Git commands: discovering a missing commit // after git clean would leave untracked user files permanently deleted. + // Other Git failures are rethrown so callers don't mistake them for a deleted checkpoint. private async commitExists(commitHash: string): Promise { try { await this.git!.raw(["cat-file", "-e", `${commitHash}^{commit}`]) return true - } catch { - return false + } catch (e) { + const message = e instanceof Error ? e.message : String(e) + if (/not a valid object name|bad object|not a commit/i.test(message)) { + return false + } + throw e } } diff --git a/src/services/checkpoints/__tests__/ShadowCheckpointService.spec.ts b/src/services/checkpoints/__tests__/ShadowCheckpointService.spec.ts index 27f7d05ecb..7e2dafc9bc 100644 --- a/src/services/checkpoints/__tests__/ShadowCheckpointService.spec.ts +++ b/src/services/checkpoints/__tests__/ShadowCheckpointService.spec.ts @@ -359,9 +359,53 @@ describe.each([[RepoPerTaskCheckpointService, "RepoPerTaskCheckpointService"]])( await fs.writeFile(laterTrackedFile, "Created after checkpoint") await service.saveCheckpoint("After checkpoint") + const unsavedFile = path.join(service.workspaceDir, "unsaved.txt") + await fs.writeFile(unsavedFile, "Never checkpointed") + await service.restoreCheckpoint(commit!.commit) expect(await fs.readFile(testFile, "utf-8")).toBe("Checkpointed content") await expect(fs.readFile(laterTrackedFile, "utf-8")).rejects.toThrow() + await expect(fs.readFile(unsavedFile, "utf-8")).rejects.toThrow() + }) + + it("does not modify the workspace when a saved checkpoint's commit object was removed", async () => { + await fs.writeFile(testFile, "Committed content") + const commit = await service.saveCheckpoint("Checkpoint to lose") + const savedHash = commit!.commit + expect(service.getCheckpoints()).toContain(savedHash) + + const objectPath = path.join( + service.checkpointsDir, + ".git", + "objects", + savedHash.slice(0, 2), + savedHash.slice(2), + ) + await fs.rm(objectPath, { force: true }) + + const untrackedFile = path.join(service.workspaceDir, "precious-untracked.txt") + await fs.writeFile(untrackedFile, "Unsaved user work") + + await expect(service.restoreCheckpoint(savedHash)).rejects.toThrow( + `Cannot restore checkpoint ${savedHash}: this checkpoint no longer exists`, + ) + expect(await fs.readFile(untrackedFile, "utf-8")).toBe("Unsaved user work") + }) + + it("reports unexpected Git failures during the commit check without changing files", async () => { + await fs.writeFile(testFile, "Committed content") + const commit = await service.saveCheckpoint("Checkpoint") + const untrackedFile = path.join(service.workspaceDir, "precious-untracked.txt") + await fs.writeFile(untrackedFile, "Unsaved user work") + + const git = service["git"]! + vitest.spyOn(git, "raw").mockRejectedValueOnce(new Error("fatal: unable to read index")) + const cleanSpy = vitest.spyOn(git, "clean") + + await expect(service.restoreCheckpoint(commit!.commit)).rejects.toThrow("fatal: unable to read index") + expect(cleanSpy).not.toHaveBeenCalled() + expect(await fs.readFile(untrackedFile, "utf-8")).toBe("Unsaved user work") + vitest.restoreAllMocks() }) it("does not create a checkpoint for ignored files", async () => { From a044e48e3e05597b74b20b958cda1079ccec32fb Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Sat, 31 Oct 2026 20:22:02 -0700 Subject: [PATCH 03/15] fix(checkpoints): surface restore failures to the user and recover safely Rethrow the original restore error from ShadowCheckpointService and checkpointRestore. On a failed delete restore, skip saving and rehydration and offer a localized Restart prompt; on a failed edit restore, discard the pending edit. Show localized, specific failure reasons for plain and completion restores. --- .../checkpoints/__tests__/checkpoint.test.ts | 84 +-- src/core/checkpoints/index.ts | 6 +- src/core/webview/ClineProvider.ts | 2 +- .../checkpointRestoreHandler.spec.ts | 461 ++++++++-------- .../webviewMessageHandler.checkpoint.spec.ts | 505 +++++++++++------- src/core/webview/checkpointRestoreHandler.ts | 30 +- src/core/webview/webviewMessageHandler.ts | 15 +- src/eslint-suppressions.json | 12 +- .../__tests__/checkpoint-translations.spec.ts | 74 +++ src/i18n/locales/ca/common.json | 6 + src/i18n/locales/de/common.json | 6 + src/i18n/locales/en/common.json | 6 + src/i18n/locales/es/common.json | 6 + src/i18n/locales/fr/common.json | 6 + src/i18n/locales/hi/common.json | 6 + src/i18n/locales/id/common.json | 6 + src/i18n/locales/it/common.json | 6 + src/i18n/locales/ja/common.json | 6 + src/i18n/locales/ko/common.json | 6 + src/i18n/locales/nl/common.json | 6 + src/i18n/locales/pl/common.json | 6 + src/i18n/locales/pt-BR/common.json | 6 + src/i18n/locales/ru/common.json | 6 + src/i18n/locales/tr/common.json | 6 + src/i18n/locales/vi/common.json | 6 + src/i18n/locales/zh-CN/common.json | 6 + src/i18n/locales/zh-TW/common.json | 6 + .../checkpoints/ShadowCheckpointService.ts | 9 +- .../__tests__/ShadowCheckpointService.spec.ts | 53 +- 29 files changed, 879 insertions(+), 480 deletions(-) create mode 100644 src/i18n/__tests__/checkpoint-translations.spec.ts diff --git a/src/core/checkpoints/__tests__/checkpoint.test.ts b/src/core/checkpoints/__tests__/checkpoint.test.ts index 9b2fc1d1aa..dcce376109 100644 --- a/src/core/checkpoints/__tests__/checkpoint.test.ts +++ b/src/core/checkpoints/__tests__/checkpoint.test.ts @@ -30,39 +30,32 @@ vi.mock("@roo-code/telemetry", () => ({ }, })) -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) => { - 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", () => { let mockProvider: any let mockTask: any let mockCheckpointService: any beforeEach(async () => { + const pathModule = await import("../../../utils/path") + const gitModule = await import("../../../utils/git") + const i18nModule = await import("../../../i18n") + vi.spyOn(pathModule, "getWorkspacePath").mockReturnValue("/test/workspace") + vi.spyOn(gitModule, "checkGitInstalled").mockResolvedValue(true) + vi.spyOn(i18nModule, "t").mockImplementation((key: string, options?: Record) => { + 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 + }) + // Create mock checkpoint service mockCheckpointService = { isInitialized: true, @@ -105,11 +98,12 @@ describe("Checkpoint functionality", () => { // Update the mock to return our mockCheckpointService const checkpointsModule = await import("../../../services/checkpoints") - vi.mocked(checkpointsModule.RepoPerTaskCheckpointService.create).mockReturnValue(mockCheckpointService) + vi.spyOn(checkpointsModule.RepoPerTaskCheckpointService, "create").mockReturnValue(mockCheckpointService) }) afterEach(() => { vi.clearAllMocks() + vi.restoreAllMocks() }) describe("checkpointSave", () => { @@ -280,18 +274,34 @@ describe("Checkpoint functionality", () => { expect(mockCheckpointService.restoreCheckpoint).not.toHaveBeenCalled() }) - it("should disable checkpoints on error", async () => { - mockCheckpointService.restoreCheckpoint.mockRejectedValue(new Error("Restore failed")) - - await checkpointRestore(mockTask, { - ts: 2, - commitHash: "abc123", - mode: "restore", - }) - - expect(mockTask.enableCheckpoints).toBe(false) - expect(mockProvider.log).toHaveBeenCalledWith("[checkpointRestore] disabling checkpoints for this task") - }) + it.each(["delete", "edit"] as const)( + "propagates restore failures for %s without rewinding", + async (operation) => { + const restoreError = new Error("Restore failed") + mockCheckpointService.restoreCheckpoint.mockRejectedValueOnce(restoreError) + const messagesBeforeRestore = [...mockTask.clineMessages] + const historyBeforeRestore = [...mockTask.apiConversationHistory] + + await expect( + checkpointRestore(mockTask, { + ts: 2, + commitHash: "abc123", + mode: "restore", + operation, + }), + ).rejects.toBe(restoreError) + + expect(mockTask.enableCheckpoints).toBe(true) + expect(mockTask.clineMessages).toEqual(messagesBeforeRestore) + expect(mockTask.apiConversationHistory).toEqual(historyBeforeRestore) + expect(mockTask.overwriteClineMessages).not.toHaveBeenCalled() + expect(mockTask.overwriteApiConversationHistory).not.toHaveBeenCalled() + expect(mockTask.say).not.toHaveBeenCalled() + expect(mockProvider.postMessageToWebview).not.toHaveBeenCalled() + expect(mockProvider.cancelTask).not.toHaveBeenCalled() + expect(mockProvider.log).toHaveBeenCalledWith("[checkpointRestore] failed to restore checkpoint") + }, + ) }) describe("checkpointDiff", () => { @@ -426,7 +436,7 @@ describe("Checkpoint functionality", () => { mockTask.checkpointService = undefined mockTask.checkpointServiceInitializing = false - getCheckpointService(mockTask) + await getCheckpointService(mockTask) const checkpointsModule = await import("../../../services/checkpoints") expect(vi.mocked(checkpointsModule.RepoPerTaskCheckpointService.create)).toHaveBeenCalledWith({ @@ -439,7 +449,7 @@ describe("Checkpoint functionality", () => { it("should disable checkpoints if workspace path is not found", async () => { const pathModule = await import("../../../utils/path") - vi.mocked(pathModule.getWorkspacePath).mockReturnValue(null as any) + vi.mocked(pathModule.getWorkspacePath).mockReturnValue("") mockTask.checkpointService = undefined mockTask.checkpointServiceInitializing = false diff --git a/src/core/checkpoints/index.ts b/src/core/checkpoints/index.ts index de424b03f7..64c50f4a15 100644 --- a/src/core/checkpoints/index.ts +++ b/src/core/checkpoints/index.ts @@ -295,9 +295,9 @@ export async function checkpointRestore( // task flow and the communication between the webview and the // `Task` instance. provider?.cancelTask() - } catch { - provider?.log("[checkpointRestore] disabling checkpoints for this task") - task.enableCheckpoints = false + } catch (error) { + provider?.log("[checkpointRestore] failed to restore checkpoint") + throw error } } diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 869a96f65d..10da3ec642 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -810,7 +810,7 @@ export class ClineProvider /** * Clears a specific pending edit operation */ - private clearPendingEditOperation(operationId: string): boolean { + public clearPendingEditOperation(operationId: string): boolean { return this.pendingEditOperations.clear(operationId) } diff --git a/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts b/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts index 21e9405125..9371cb21f9 100644 --- a/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts +++ b/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts @@ -1,243 +1,278 @@ -import { describe, it, expect, vi, beforeEach } from "vitest" -import { handleCheckpointRestoreOperation } from "../checkpointRestoreHandler" -import { saveTaskMessages } from "../../task-persistence" -import pWaitFor from "p-wait-for" import * as vscode from "vscode" +import type { HistoryItem } from "@roo-code/types" + +import { Task } from "../../task/Task" +import { ClineProvider } from "../ClineProvider" +import { handleCheckpointRestoreOperation, type CheckpointRestoreConfig } from "../checkpointRestoreHandler" +import { PendingEditOperationStore } from "../PendingEditOperationStore" +import { saveTaskMessages } from "../../task-persistence" +import { t } from "../../../i18n" -// Mock dependencies +const { constructTask, showWarningMessage } = vi.hoisted(() => ({ + constructTask: vi.fn<(...arguments_: unknown[]) => object>(), + showWarningMessage: vi.fn<(message: string, ...items: string[]) => Promise>(), +})) +vi.mock("../../task/Task", async (importOriginal) => ({ + ...(await importOriginal()), + Task: vi.fn(function (...arguments_: unknown[]) { + return constructTask(...arguments_) + }), +})) vi.mock("../../task-persistence", async (importOriginal) => ({ ...(await importOriginal()), saveTaskMessages: vi.fn(), })) -vi.mock("p-wait-for") -vi.mock("vscode", () => ({ - window: { - showErrorMessage: vi.fn(), - }, +vi.mock("p-wait-for", () => ({ + default: vi.fn(async (condition: () => boolean | PromiseLike) => { + if (!(await condition())) throw new Error("condition not met") + }), +})) +vi.mock("vscode", async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + window: { ...actual.window, showErrorMessage: vi.fn(), showWarningMessage }, + } +}) +vi.mock("../../../i18n", () => ({ + t: vi.fn((key: string, options?: { error?: string }) => + options?.error === undefined ? key : `${key}: ${options.error}`, + ), })) -describe("checkpointRestoreHandler", () => { - let mockProvider: any - let mockCline: any +function createFixture() { + const taskDouble = { + taskId: "test-task-123", + abort: false, + clineMessages: [ + { ts: 1, type: "say", say: "text", text: "Initial task" }, + { ts: 3, type: "say", say: "user_feedback", text: "Original prompt" }, + { ts: 4, type: "say", say: "checkpoint_saved", text: "abc123" }, + ] satisfies Task["clineMessages"], + apiConversationHistory: [ + { ts: 3, role: "user", content: [{ type: "text", text: "Original prompt" }] }, + ] satisfies Task["apiConversationHistory"], + abortTask: vi.fn(async () => { + taskDouble.abort = true + }), + checkpointRestore: vi.fn(), + overwriteClineMessages: vi.fn(), + overwriteApiConversationHistory: vi.fn(), + handleWebviewAskResponse: vi.fn(), + emit: vi.fn(), + } + // Supplies restore, abort, history, and pending-edit application APIs without a running request loop. + const task = taskDouble as unknown as Task + constructTask.mockReturnValue(task) + const historyItem: HistoryItem = { + id: task.taskId, + number: 1, + ts: 1, + task: "Initial task", + tokensIn: 0, + tokensOut: 0, + totalCost: 0, + } + const store = new PendingEditOperationStore(60_000, vi.fn()) + const providerDouble = { + pendingEditOperations: store, + taskRegistry: { current: undefined }, + performPreparationTasks: vi.fn<(task: Task) => Promise>(), + log: vi.fn<(message: string) => void>(), + getCurrentTask: vi.fn(() => task), + getTaskWithId: vi.fn().mockResolvedValue({ + historyItem, + taskDirPath: "/test/task", + apiConversationHistoryFilePath: "/test/api.json", + uiMessagesFilePath: "/test/messages.json", + apiConversationHistory: [], + }), + createTaskWithHistoryItem: vi.fn().mockResolvedValue(task), + setPendingEditOperation: vi.fn(), + clearPendingEditOperation: vi.fn(), + getPendingEditOperation: ClineProvider.prototype["getPendingEditOperation"], + getState: vi.fn(async () => ({ apiConfiguration: {} })), + contextProxy: { globalStorageUri: { fsPath: "/test/storage" } }, + } + // Supplies restore/rehydration dependencies and the real store; scheduling is disabled in the rehydration test. + const provider = providerDouble as unknown as ClineProvider + providerDouble.setPendingEditOperation.mockImplementation((operationId, editData) => + ClineProvider.prototype.setPendingEditOperation.call(provider, operationId, editData), + ) + providerDouble.clearPendingEditOperation.mockImplementation((operationId) => + ClineProvider.prototype.clearPendingEditOperation.call(provider, operationId), + ) + const config: CheckpointRestoreConfig = { + provider, + currentCline: task, + messageTs: 3, + messageIndex: 1, + checkpoint: { hash: "abc123" }, + operation: "delete", + } + return { task, taskDouble, provider, providerDouble, historyItem, store, config } +} +describe("checkpointRestoreHandler", () => { + let fixture: ReturnType beforeEach(() => { vi.clearAllMocks() + showWarningMessage.mockResolvedValue(undefined) + fixture = createFixture() + vi.spyOn(console, "error").mockImplementation(() => {}) + }) + afterEach(() => { + fixture.store.clearAll() + vi.useRealTimers() + vi.restoreAllMocks() + }) - // Setup mock Cline instance - mockCline = { - taskId: "test-task-123", - abort: false, - abortTask: vi.fn(() => { - mockCline.abort = true - }), - checkpointRestore: vi.fn(), - clineMessages: [ - { ts: 1, type: "user", say: "user", text: "First message" }, - { ts: 2, type: "assistant", say: "assistant", text: "Response" }, - { - ts: 3, - type: "user", - say: "user", - text: "Checkpoint message", - checkpoint: { hash: "abc123" }, - }, - { ts: 4, type: "assistant", say: "assistant", text: "After checkpoint" }, - ], - } - - // Setup mock provider - mockProvider = { - getCurrentTask: vi.fn(() => mockCline), - postMessageToWebview: vi.fn(), - getTaskWithId: vi.fn(() => ({ - historyItem: { id: "test-task-123", messages: mockCline.clineMessages }, - })), - createTaskWithHistoryItem: vi.fn(), - setPendingEditOperation: vi.fn(), - contextProxy: { - globalStorageUri: { fsPath: "/test/storage" }, - }, - } - - // Mock pWaitFor to resolve immediately - ;(pWaitFor as any).mockImplementation(async (_condition: () => boolean) => { - // Simulate the condition being met - return Promise.resolve() + it.each([false, true])("successful delete saves and rehydrates, already aborted: %s", async (aborted) => { + fixture.taskDouble.abort = aborted + fixture.taskDouble.checkpointRestore.mockImplementationOnce(async () => { + fixture.task.clineMessages = fixture.task.clineMessages.slice(0, 1) + }) + await handleCheckpointRestoreOperation(fixture.config) + expect(fixture.taskDouble.abortTask).toHaveBeenCalledTimes(aborted ? 0 : 1) + expect(fixture.taskDouble.checkpointRestore).toHaveBeenCalledExactlyOnceWith({ + ts: 3, + commitHash: "abc123", + mode: "restore", + operation: "delete", }) + expect(saveTaskMessages).toHaveBeenCalledExactlyOnceWith({ + messages: fixture.task.clineMessages, + taskId: fixture.task.taskId, + globalStoragePath: "/test/storage", + }) + expect(fixture.providerDouble.getTaskWithId).toHaveBeenCalledExactlyOnceWith(fixture.task.taskId) + expect(fixture.providerDouble.createTaskWithHistoryItem).toHaveBeenCalledExactlyOnceWith(fixture.historyItem) + expect(vscode.window.showErrorMessage).not.toHaveBeenCalled() }) - describe("handleCheckpointRestoreOperation", () => { - it("should abort task before checkpoint restore for delete operations", async () => { - // Simulate a task that hasn't been aborted yet - mockCline.abort = false - - await handleCheckpointRestoreOperation({ - provider: mockProvider, - currentCline: mockCline, - messageTs: 3, - messageIndex: 2, - checkpoint: { hash: "abc123" }, - operation: "delete", - }) - - // Verify abortTask was called before checkpointRestore - expect(mockCline.abortTask).toHaveBeenCalled() - expect(mockCline.checkpointRestore).toHaveBeenCalled() - - // Verify the order of operations - const abortOrder = mockCline.abortTask.mock.invocationCallOrder[0] - const restoreOrder = mockCline.checkpointRestore.mock.invocationCallOrder[0] - expect(abortOrder).toBeLessThan(restoreOrder) + it("successful edit retains pending data for automatic rehydration, without saving or aborting", async () => { + await handleCheckpointRestoreOperation({ + ...fixture.config, + operation: "edit", + editData: { editedContent: "Edited prompt", images: ["image.png"], apiConversationHistoryIndex: 0 }, }) - - it("should not abort task if already aborted", async () => { - // Simulate a task that's already aborted - mockCline.abort = true - - await handleCheckpointRestoreOperation({ - provider: mockProvider, - currentCline: mockCline, - messageTs: 3, - messageIndex: 2, - checkpoint: { hash: "abc123" }, - operation: "delete", - }) - - // Verify abortTask was not called - expect(mockCline.abortTask).not.toHaveBeenCalled() - expect(mockCline.checkpointRestore).toHaveBeenCalled() + expect(fixture.store.get(`task-${fixture.task.taskId}`)).toEqual({ + messageTs: 3, + editedContent: "Edited prompt", + images: ["image.png"], + messageIndex: 1, + apiConversationHistoryIndex: 0, }) - - it("should handle edit operations with pending edit data", async () => { - const editData = { - editedContent: "Edited content", - images: ["image1.png"], - apiConversationHistoryIndex: 2, - } - - await handleCheckpointRestoreOperation({ - provider: mockProvider, - currentCline: mockCline, - messageTs: 3, - messageIndex: 2, - checkpoint: { hash: "abc123" }, - operation: "edit", - editData, - }) - - // Verify abortTask was NOT called for edit operations - expect(mockCline.abortTask).not.toHaveBeenCalled() - - // Verify pending edit operation was set - expect(mockProvider.setPendingEditOperation).toHaveBeenCalledWith("task-test-task-123", { - messageTs: 3, - editedContent: "Edited content", - images: ["image1.png"], - messageIndex: 2, - apiConversationHistoryIndex: 2, - }) - - // Verify checkpoint restore was called with edit operation - expect(mockCline.checkpointRestore).toHaveBeenCalledWith({ - ts: 3, - commitHash: "abc123", - mode: "restore", - operation: "edit", - }) + expect(fixture.taskDouble.checkpointRestore).toHaveBeenCalledWith({ + ts: 3, + commitHash: "abc123", + mode: "restore", + operation: "edit", }) + expect(fixture.taskDouble.abortTask).not.toHaveBeenCalled() + expect(saveTaskMessages).not.toHaveBeenCalled() + expect(fixture.providerDouble.createTaskWithHistoryItem).not.toHaveBeenCalled() + expect(fixture.providerDouble.clearPendingEditOperation).not.toHaveBeenCalled() + }) - it("should save messages after delete operation", async () => { - // Mock the checkpoint restore to simulate message deletion - mockCline.checkpointRestore.mockImplementation(async () => { - mockCline.clineMessages = mockCline.clineMessages.slice(0, 2) - }) - - await handleCheckpointRestoreOperation({ - provider: mockProvider, - currentCline: mockCline, - messageTs: 3, - messageIndex: 2, - checkpoint: { hash: "abc123" }, - operation: "delete", - }) - - // Verify saveTaskMessages was called - expect(saveTaskMessages).toHaveBeenCalledWith({ - messages: mockCline.clineMessages, - taskId: "test-task-123", - globalStoragePath: "/test/storage", - }) - - // Verify createTaskWithHistoryItem was called - expect(mockProvider.createTaskWithHistoryItem).toHaveBeenCalled() - }) + it.each([ + { operation: "delete", failure: new Error("Missing commit"), reason: "Missing commit" }, + { operation: "delete", failure: "Invalid reference", reason: "Invalid reference" }, + { operation: "edit", failure: new Error("Missing commit"), reason: "Missing commit" }, + { operation: "edit", failure: "Invalid reference", reason: "Invalid reference" }, + ] as const)( + "$operation reports and rethrows the original failure: $reason", + async ({ operation, failure, reason }) => { + const messages = structuredClone(fixture.task.clineMessages) + const history = structuredClone(fixture.task.apiConversationHistory) + fixture.taskDouble.checkpointRestore.mockRejectedValueOnce(failure) + await expect( + handleCheckpointRestoreOperation({ + ...fixture.config, + operation, + editData: { editedContent: "Edited prompt", apiConversationHistoryIndex: 0 }, + }), + ).rejects.toBe(failure) - it("should reinitialize task with correct history item after delete", async () => { - const expectedHistoryItem = { - id: "test-task-123", - messages: mockCline.clineMessages, + expect(t).toHaveBeenCalledWith("common:errors.checkpoint_restore_failed", { error: reason }) + expect(vscode.window.showErrorMessage).toHaveBeenCalledExactlyOnceWith( + `common:errors.checkpoint_restore_failed: ${reason}`, + ) + expect(console.error).toHaveBeenCalledWith(`Error in checkpoint restore (${operation}):`, failure) + expect(fixture.task.clineMessages).toEqual(messages) + expect(fixture.task.apiConversationHistory).toEqual(history) + expect(saveTaskMessages).not.toHaveBeenCalled() + expect(fixture.providerDouble.getTaskWithId).not.toHaveBeenCalled() + expect(fixture.providerDouble.createTaskWithHistoryItem).not.toHaveBeenCalled() + expect(fixture.store.get(`task-${fixture.task.taskId}`)).toBeUndefined() + if (operation === "delete") { + expect(showWarningMessage).toHaveBeenCalledWith( + "common:checkpoints.restart_after_restore_failure", + "common:checkpoints.restart", + ) + } else { + expect(showWarningMessage).not.toHaveBeenCalled() + expect(fixture.providerDouble.clearPendingEditOperation).toHaveBeenCalledWith( + `task-${fixture.task.taskId}`, + ) } - - await handleCheckpointRestoreOperation({ - provider: mockProvider, - currentCline: mockCline, - messageTs: 3, - messageIndex: 2, - checkpoint: { hash: "abc123" }, - operation: "delete", - }) - - // Verify getTaskWithId was called - expect(mockProvider.getTaskWithId).toHaveBeenCalledWith("test-task-123") - - // Verify createTaskWithHistoryItem was called with the correct history item - expect(mockProvider.createTaskWithHistoryItem).toHaveBeenCalledWith(expectedHistoryItem) + }, + ) + + it("does not restart a different current task when the user accepts", async () => { + fixture.taskDouble.checkpointRestore.mockRejectedValueOnce(new Error("Missing commit")) + showWarningMessage.mockImplementationOnce(async () => { + fixture.providerDouble.getCurrentTask.mockReturnValue(undefined) + return "common:checkpoints.restart" }) + await expect(handleCheckpointRestoreOperation(fixture.config)).rejects.toThrow("Missing commit") + expect(fixture.providerDouble.getTaskWithId).not.toHaveBeenCalled() + expect(fixture.providerDouble.createTaskWithHistoryItem).not.toHaveBeenCalled() + expect(saveTaskMessages).not.toHaveBeenCalled() + }) - it("should not save messages or reinitialize for edit operation", async () => { - const editData = { - editedContent: "Edited content", - images: [], - apiConversationHistoryIndex: 2, - } - - await handleCheckpointRestoreOperation({ - provider: mockProvider, - currentCline: mockCline, - messageTs: 3, - messageIndex: 2, - checkpoint: { hash: "abc123" }, + it("does not apply a failed pending edit on later real provider rehydration", async () => { + vi.useFakeTimers() + const messages = structuredClone(fixture.task.clineMessages) + const history = structuredClone(fixture.task.apiConversationHistory) + fixture.taskDouble.checkpointRestore.mockRejectedValueOnce(new Error("Missing commit")) + await expect( + handleCheckpointRestoreOperation({ + ...fixture.config, operation: "edit", - editData, - }) - - // Verify saveTaskMessages was NOT called for edit operation - expect(saveTaskMessages).not.toHaveBeenCalled() + editData: { editedContent: "Must not be applied", apiConversationHistoryIndex: 0 }, + }), + ).rejects.toThrow("Missing commit") + expect(fixture.store.get(`task-${fixture.task.taskId}`)).toBeUndefined() - // Verify createTaskWithHistoryItem was NOT called for edit operation - expect(mockProvider.createTaskWithHistoryItem).not.toHaveBeenCalled() + await ClineProvider.prototype.createTaskWithHistoryItem.call(fixture.provider, fixture.historyItem, { + startTask: false, }) + await vi.advanceTimersByTimeAsync(100) + expect(Task).toHaveBeenCalledOnce() + expect(fixture.providerDouble.performPreparationTasks).toHaveBeenCalledWith(fixture.task) + expect(fixture.taskDouble.overwriteClineMessages).not.toHaveBeenCalled() + expect(fixture.taskDouble.overwriteApiConversationHistory).not.toHaveBeenCalled() + expect(fixture.taskDouble.handleWebviewAskResponse).not.toHaveBeenCalled() + expect(fixture.task.clineMessages).toEqual(messages) + expect(fixture.task.apiConversationHistory).toEqual(history) + expect(saveTaskMessages).not.toHaveBeenCalled() + }) - it("should handle errors gracefully", async () => { - // Mock checkpoint restore to throw an error - mockCline.checkpointRestore.mockRejectedValue(new Error("Checkpoint restore failed")) - - // The function should throw and show an error message - await expect( - handleCheckpointRestoreOperation({ - provider: mockProvider, - currentCline: mockCline, - messageTs: 3, - messageIndex: 2, - checkpoint: { hash: "abc123" }, - operation: "delete", - }), - ).rejects.toThrow("Checkpoint restore failed") - - // Verify error message was shown - expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( - "Error during checkpoint restore: Checkpoint restore failed", - ) + it("applies a successful pending edit through the same real provider rehydration path", async () => { + vi.useFakeTimers() + await handleCheckpointRestoreOperation({ + ...fixture.config, + operation: "edit", + editData: { editedContent: "Accepted edit", images: [], apiConversationHistoryIndex: 0 }, + }) + await ClineProvider.prototype.createTaskWithHistoryItem.call(fixture.provider, fixture.historyItem, { + startTask: false, }) + await vi.advanceTimersByTimeAsync(100) + expect(fixture.taskDouble.overwriteClineMessages).toHaveBeenCalledWith(fixture.task.clineMessages.slice(0, 1)) + expect(fixture.taskDouble.overwriteApiConversationHistory).toHaveBeenCalledWith([]) + expect(fixture.taskDouble.handleWebviewAskResponse).toHaveBeenCalledWith("messageResponse", "Accepted edit", []) + expect(fixture.store.get(`task-${fixture.task.taskId}`)).toBeUndefined() }) }) diff --git a/src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts index 71a58d6f8d..bfdb941adc 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts @@ -1,256 +1,357 @@ -import { describe, it, expect, vi, beforeEach } from "vitest" import pWaitFor from "p-wait-for" +import * as vscode from "vscode" +import type { HistoryItem, WebviewMessage } from "@roo-code/types" + +import type { Task } from "../../task/Task" import { webviewMessageHandler } from "../webviewMessageHandler" +import { ClineProvider } from "../ClineProvider" +import { PendingEditOperationStore } from "../PendingEditOperationStore" import { saveTaskMessages } from "../../task-persistence" -import { handleCheckpointRestoreOperation } from "../checkpointRestoreHandler" import { MessageManager } from "../../message-manager" +import { t } from "../../../i18n" + +const { showWarningMessage } = vi.hoisted(() => ({ + showWarningMessage: vi.fn<(message: string, ...items: string[]) => Promise>(), +})) -// Mock dependencies vi.mock("../../task-persistence", async (importOriginal) => ({ ...(await importOriginal()), saveTaskMessages: vi.fn(), })) -vi.mock("../checkpointRestoreHandler") vi.mock("p-wait-for", () => ({ - default: vi.fn(async (condition: () => boolean) => { - if (!condition()) { - throw new Error("condition not met") - } + default: vi.fn(async (condition: () => boolean | PromiseLike) => { + if (!(await condition())) throw new Error("condition not met") }), })) -vi.mock("vscode", () => ({ - window: { - showErrorMessage: vi.fn(), - }, - workspace: { - workspaceFolders: undefined, - }, +vi.mock("vscode", async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + window: { ...actual.window, showErrorMessage: vi.fn(), showWarningMessage }, + } +}) +vi.mock("../../../i18n", () => ({ + t: vi.fn((key: string, options?: { error?: string }) => + options?.error === undefined ? key : `${key}: ${options.error}`, + ), })) +function createFixture() { + const taskDouble = { + taskId: "test-task-123", + isInitialized: true, + abort: false, + clineMessages: [ + { ts: 1, type: "say", say: "text", text: "Initial task" }, + { ts: 2, type: "say", say: "checkpoint_saved", text: "initial-checkpoint" }, + { ts: 3, type: "say", say: "user_feedback", text: "Latest prompt" }, + { ts: 4, type: "say", say: "checkpoint_saved", text: "latest-prompt-checkpoint" }, + { ts: 5, type: "say", say: "completion_result", text: "Task complete" }, + { ts: 6, type: "ask", ask: "completion_result", text: "", partial: false }, + ] satisfies Task["clineMessages"], + apiConversationHistory: [ + { ts: 1, role: "user", content: [{ type: "text", text: "Initial task" }] }, + { ts: 3, role: "user", content: [{ type: "text", text: "Latest prompt" }] }, + { ts: 5, role: "assistant", content: [{ type: "text", text: "Task complete" }] }, + ] satisfies Task["apiConversationHistory"], + abortTask: vi.fn(async () => { + taskDouble.abort = true + }), + checkpointDiff: vi.fn(), + checkpointRestore: vi.fn(), + overwriteClineMessages: vi.fn(), + overwriteApiConversationHistory: vi.fn(), + handleWebviewAskResponse: vi.fn(), + } + // Supplies checkpoint, abort, message/history, and overwrite APIs without starting a real task. + const task = taskDouble as unknown as Task + Object.defineProperty(task, "messageManager", { value: new MessageManager(task) }) + const historyItem: HistoryItem = { + id: task.taskId, + number: 1, + ts: 1, + task: "Initial task", + tokensIn: 0, + tokensOut: 0, + totalCost: 0, + } + const store = new PendingEditOperationStore(60_000, vi.fn()) + const providerDouble = { + pendingEditOperations: store, + getState: vi.fn( + async () => + ({ + maxImageFileSize: 5, + maxTotalImageSize: 20, + }) satisfies Pick< + Awaited>, + "maxImageFileSize" | "maxTotalImageSize" + >, + ), + getCurrentTask: vi.fn(() => task), + postMessageToWebview: vi.fn(), + postStateToWebview: vi.fn(), + getTaskWithId: vi.fn().mockResolvedValue({ + historyItem, + taskDirPath: "/test/task", + apiConversationHistoryFilePath: "/test/api.json", + uiMessagesFilePath: "/test/messages.json", + apiConversationHistory: [], + }), + createTaskWithHistoryItem: vi.fn().mockResolvedValue(task), + setPendingEditOperation: vi.fn(), + clearPendingEditOperation: vi.fn(), + cancelTask: vi.fn(), + contextProxy: { globalStorageUri: { fsPath: "/test/storage" } }, + } + // Supplies the handler's provider APIs and the real pending-edit store, without extension activation. + const provider = providerDouble as unknown as ClineProvider + providerDouble.setPendingEditOperation.mockImplementation((operationId, editData) => + ClineProvider.prototype.setPendingEditOperation.call(provider, operationId, editData), + ) + providerDouble.clearPendingEditOperation.mockImplementation((operationId) => + ClineProvider.prototype.clearPendingEditOperation.call(provider, operationId), + ) + return { task, taskDouble, provider, providerDouble, historyItem, store } +} + describe("webviewMessageHandler - checkpoint operations", () => { - let mockProvider: any - let mockCline: any + let fixture: ReturnType beforeEach(() => { vi.clearAllMocks() + fixture = createFixture() + vi.spyOn(console, "error").mockImplementation(() => {}) + }) - // Setup mock Cline instance - mockCline = { - taskId: "test-task-123", - isInitialized: true, - clineMessages: [ - { ts: 1, type: "user", say: "user", text: "First message" }, - { ts: 2, type: "assistant", say: "checkpoint_saved", text: "abc123" }, - { ts: 3, type: "user", say: "user", text: "Message to delete" }, - { ts: 4, type: "assistant", say: "assistant", text: "After message" }, - ], - apiConversationHistory: [ - { ts: 1, role: "user", content: [{ type: "text", text: "First message" }] }, - { ts: 3, role: "user", content: [{ type: "text", text: "Message to delete" }] }, - { ts: 4, role: "assistant", content: [{ type: "text", text: "After message" }] }, - ], - checkpointDiff: vi.fn(), - checkpointRestore: vi.fn(), - overwriteClineMessages: vi.fn(), - overwriteApiConversationHistory: vi.fn(), - } - mockCline.messageManager = new MessageManager(mockCline) + afterEach(() => { + fixture.store.clearAll() + vi.restoreAllMocks() + }) - // Setup mock provider - mockProvider = { - getCurrentTask: vi.fn(() => mockCline), - postMessageToWebview: vi.fn(), - getTaskWithId: vi.fn(() => ({ - historyItem: { id: "test-task-123", messages: mockCline.clineMessages }, - })), - createTaskWithHistoryItem: vi.fn(), - setPendingEditOperation: vi.fn(), - cancelTask: vi.fn(), - contextProxy: { - globalStorageUri: { fsPath: "/test/storage" }, - }, - getState: vi.fn().mockResolvedValue({ - maxImageFileSize: 5, - maxTotalImageSize: 20, - }), + function expectUnchanged(messages: Task["clineMessages"], history: Task["apiConversationHistory"]) { + expect(fixture.task.clineMessages).toEqual(messages) + expect(fixture.task.apiConversationHistory).toEqual(history) + expect(fixture.taskDouble.overwriteClineMessages).not.toHaveBeenCalled() + expect(fixture.taskDouble.overwriteApiConversationHistory).not.toHaveBeenCalled() + expect(fixture.taskDouble.handleWebviewAskResponse).not.toHaveBeenCalled() + expect(saveTaskMessages).not.toHaveBeenCalled() + } + + it.each([true, false])("failed delete restarts only after accepting the prompt: %s", async (accept) => { + const { task, taskDouble, provider, providerDouble, historyItem } = fixture + const messages = structuredClone(task.clineMessages) + const history = structuredClone(task.apiConversationHistory) + const restoreError = new Error("Checkpoint commit is missing") + taskDouble.checkpointRestore.mockRejectedValueOnce(restoreError) + let answerPrompt: (answer: string | undefined) => void = () => { + throw new Error("Prompt not shown") + } + const promptShown = new Promise((resolve) => { + showWarningMessage.mockImplementationOnce( + () => + new Promise((answer) => { + answerPrompt = answer + resolve() + }), + ) + }) + const operation = webviewMessageHandler(provider, { + type: "deleteMessageConfirm", + messageTs: 3, + restoreCheckpoint: true, + }) + await promptShown + + expect(taskDouble.abortTask).toHaveBeenCalledOnce() + expect(taskDouble.checkpointRestore).toHaveBeenCalledWith({ + ts: 3, + commitHash: "latest-prompt-checkpoint", + mode: "restore", + operation: "delete", + }) + expect(taskDouble.abortTask.mock.invocationCallOrder[0]).toBeLessThan( + taskDouble.checkpointRestore.mock.invocationCallOrder[0], + ) + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( + "common:errors.checkpoint_restore_failed: Checkpoint commit is missing", + ) + expect(t).toHaveBeenCalledWith("common:errors.checkpoint_restore_failed", { error: restoreError.message }) + expect(t).toHaveBeenCalledWith("common:checkpoints.restart_after_restore_failure") + expect(t).toHaveBeenCalledWith("common:checkpoints.restart") + expect(vscode.window.showWarningMessage).toHaveBeenCalledWith( + "common:checkpoints.restart_after_restore_failure", + "common:checkpoints.restart", + ) + expect(providerDouble.getTaskWithId).not.toHaveBeenCalled() + expect(providerDouble.createTaskWithHistoryItem).not.toHaveBeenCalled() + expectUnchanged(messages, history) + + answerPrompt(accept ? "common:checkpoints.restart" : undefined) + await operation + if (accept) { + expect(providerDouble.getTaskWithId).toHaveBeenCalledExactlyOnceWith(task.taskId) + expect(providerDouble.createTaskWithHistoryItem).toHaveBeenCalledExactlyOnceWith(historyItem) + } else { + expect(providerDouble.getTaskWithId).not.toHaveBeenCalled() + expect(providerDouble.createTaskWithHistoryItem).not.toHaveBeenCalled() } + expectUnchanged(messages, history) + expect(vscode.window.showErrorMessage).toHaveBeenCalledTimes(2) + expect(vscode.window.showErrorMessage).toHaveBeenLastCalledWith( + "common:errors.message.error_deleting_message: Checkpoint commit is missing", + ) }) - describe("delete operations with checkpoint restoration", () => { - it("should call handleCheckpointRestoreOperation for checkpoint deletes", async () => { - // Mock handleCheckpointRestoreOperation - ;(handleCheckpointRestoreOperation as any).mockResolvedValue(undefined) - - // Call the handler with delete confirmation - await webviewMessageHandler(mockProvider, { - type: "deleteMessageConfirm", - messageTs: 1, - restoreCheckpoint: true, - }) - - // Verify handleCheckpointRestoreOperation was called with correct parameters - expect(handleCheckpointRestoreOperation).toHaveBeenCalledWith({ - provider: mockProvider, - currentCline: mockCline, - messageTs: 1, - messageIndex: 0, - checkpoint: { hash: "abc123" }, - operation: "delete", + it("failed edit clears the real pending operation without applying or saving the edit", async () => { + const { task, taskDouble, provider, providerDouble, store } = fixture + const messages = structuredClone(task.clineMessages) + const history = structuredClone(task.apiConversationHistory) + const operationId = `task-${task.taskId}` + taskDouble.checkpointRestore.mockImplementationOnce(async () => { + expect(store.get(operationId)).toEqual({ + messageTs: 3, + editedContent: "Edited prompt", + images: [], + messageIndex: 2, + apiConversationHistoryIndex: 1, }) + throw new Error("Checkpoint belongs to another task") + }) + await webviewMessageHandler(provider, { + type: "editMessageConfirm", + messageTs: 3, + text: "Edited prompt", + restoreCheckpoint: true, }) - it("should save messages for non-checkpoint deletes", async () => { - // Call the handler with delete confirmation (no checkpoint restoration) - await webviewMessageHandler(mockProvider, { - type: "deleteMessageConfirm", - messageTs: 2, - restoreCheckpoint: false, - }) - - // Verify saveTaskMessages was called - expect(saveTaskMessages).toHaveBeenCalledWith({ - messages: expect.any(Array), - taskId: "test-task-123", - globalStoragePath: "/test/storage", - }) - - // Verify checkpoint restore was NOT called - expect(mockCline.checkpointRestore).not.toHaveBeenCalled() + expect(taskDouble.checkpointRestore).toHaveBeenCalledExactlyOnceWith({ + ts: 3, + commitHash: "latest-prompt-checkpoint", + mode: "restore", + operation: "edit", + }) + expect(t).toHaveBeenCalledWith("common:errors.checkpoint_restore_failed", { + error: "Checkpoint belongs to another task", }) + expect(providerDouble.getTaskWithId).not.toHaveBeenCalled() + expect(providerDouble.clearPendingEditOperation).toHaveBeenCalledExactlyOnceWith(operationId) + expect(store.get(operationId)).toBeUndefined() + expect(taskDouble.abortTask).not.toHaveBeenCalled() + expect(providerDouble.createTaskWithHistoryItem).not.toHaveBeenCalled() + expect(vscode.window.showWarningMessage).not.toHaveBeenCalled() + expect(vscode.window.showErrorMessage).toHaveBeenCalledTimes(2) + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( + "common:errors.checkpoint_restore_failed: Checkpoint belongs to another task", + ) + expect(vscode.window.showErrorMessage).toHaveBeenLastCalledWith( + "common:errors.message.error_editing_message: Checkpoint belongs to another task", + ) + expectUnchanged(messages, history) }) - describe("edit operations with checkpoint restoration", () => { - it("should call handleCheckpointRestoreOperation for checkpoint edits", async () => { - // Mock handleCheckpointRestoreOperation - ;(handleCheckpointRestoreOperation as any).mockResolvedValue(undefined) - - // Call the handler with edit confirmation - await webviewMessageHandler(mockProvider, { - type: "editMessageConfirm", - messageTs: 1, - text: "Edited checkpoint message", - restoreCheckpoint: true, - }) - - // Verify handleCheckpointRestoreOperation was called with correct parameters - expect(handleCheckpointRestoreOperation).toHaveBeenCalledWith({ - provider: mockProvider, - currentCline: mockCline, - messageTs: 1, - messageIndex: 0, - checkpoint: { hash: "abc123" }, - operation: "edit", - editData: { - editedContent: "Edited checkpoint message", - images: [], - apiConversationHistoryIndex: 0, - }, - }) + it("saves non-checkpoint deletes without restoring", async () => { + await webviewMessageHandler(fixture.provider, { + type: "deleteMessageConfirm", + messageTs: 3, + restoreCheckpoint: false, }) + expect(saveTaskMessages).toHaveBeenCalledWith({ + messages: expect.any(Array), + taskId: fixture.task.taskId, + globalStoragePath: "/test/storage", + }) + expect(fixture.taskDouble.checkpointRestore).not.toHaveBeenCalled() }) - describe("completion checkpoint actions", () => { - beforeEach(() => { - mockCline.clineMessages = [ - { ts: 1, type: "say", say: "text", text: "Initial task" }, - { ts: 2, type: "say", say: "checkpoint_saved", text: "initial-checkpoint" }, - { ts: 3, type: "say", say: "user_feedback", text: "Latest prompt" }, - { ts: 4, type: "say", say: "checkpoint_saved", text: "latest-prompt-checkpoint" }, - { ts: 5, type: "say", say: "completion_result", text: "Task complete" }, - { ts: 6, type: "ask", ask: "completion_result", text: "", partial: false }, - ] + it.each([ + { + type: "checkpointRestore", + failure: new Error("Missing checkpoint commit"), + reason: "Missing checkpoint commit", + }, + { type: "checkpointRestore", failure: "Invalid checkpoint reference", reason: "Invalid checkpoint reference" }, + { + type: "completionCheckpointRestore", + failure: new Error("Missing checkpoint commit"), + reason: "Missing checkpoint commit", + }, + { + type: "completionCheckpointRestore", + failure: "Invalid checkpoint reference", + reason: "Invalid checkpoint reference", + }, + ] as const)("$type localizes the specific restore failure: $reason", async ({ type, failure, reason }) => { + const messages = structuredClone(fixture.task.clineMessages) + const history = structuredClone(fixture.task.apiConversationHistory) + fixture.taskDouble.checkpointRestore.mockRejectedValueOnce(failure) + const message: WebviewMessage = + type === "checkpointRestore" + ? { type, payload: { ts: 4, commitHash: "latest-prompt-checkpoint", mode: "restore" } } + : { type } + await webviewMessageHandler(fixture.provider, message) + + expect(fixture.providerDouble.cancelTask).toHaveBeenCalledOnce() + expect(fixture.taskDouble.checkpointRestore).toHaveBeenCalledWith({ + ts: 4, + commitHash: "latest-prompt-checkpoint", + mode: "restore", }) + expect(console.error).toHaveBeenCalledWith(`[${type}] checkpointRestore failed:`, failure) + expect(t).toHaveBeenCalledExactlyOnceWith("common:errors.checkpoint_restore_failed", { error: reason }) + expect(vscode.window.showErrorMessage).toHaveBeenCalledExactlyOnceWith( + `common:errors.checkpoint_restore_failed: ${reason}`, + ) + expect(fixture.providerDouble.createTaskWithHistoryItem).not.toHaveBeenCalled() + expectUnchanged(messages, history) + }) - it("diffs changes from the checkpoint created after the latest prompt", async () => { - await webviewMessageHandler(mockProvider, { type: "completionCheckpointDiff" }) - - expect(mockCline.checkpointDiff).toHaveBeenCalledWith({ + describe("completion checkpoint actions", () => { + it("diffs the checkpoint created after the latest prompt", async () => { + await webviewMessageHandler(fixture.provider, { type: "completionCheckpointDiff" }) + expect(fixture.taskDouble.checkpointDiff).toHaveBeenCalledWith({ ts: 4, commitHash: "latest-prompt-checkpoint", mode: "to-current", }) }) - it("restores files and task state to the checkpoint created after the latest prompt", async () => { - const callOrder: string[] = [] - mockProvider.cancelTask.mockImplementation(async () => callOrder.push("cancelTask")) - mockCline.checkpointRestore.mockImplementation(async () => callOrder.push("checkpointRestore")) - - await webviewMessageHandler(mockProvider, { type: "completionCheckpointRestore" }) - - expect(mockProvider.cancelTask).toHaveBeenCalled() - expect(mockCline.checkpointRestore).toHaveBeenCalledWith({ + it("cancels before restoring the latest prompt checkpoint", async () => { + await webviewMessageHandler(fixture.provider, { type: "completionCheckpointRestore" }) + expect(fixture.taskDouble.checkpointRestore).toHaveBeenCalledWith({ ts: 4, commitHash: "latest-prompt-checkpoint", mode: "restore", }) - expect(callOrder).toEqual(["cancelTask", "checkpointRestore"]) - }) - - it("does not diff or restore when no latest-prompt checkpoint exists", async () => { - mockCline.clineMessages = [ - { ts: 1, type: "say", say: "text", text: "Initial task" }, - { ts: 2, type: "say", say: "user_feedback", text: "Latest prompt" }, - { ts: 3, type: "ask", ask: "completion_result", text: "", partial: false }, - ] - - await webviewMessageHandler(mockProvider, { type: "completionCheckpointDiff" }) - await webviewMessageHandler(mockProvider, { type: "completionCheckpointRestore" }) - - expect(mockCline.checkpointDiff).not.toHaveBeenCalled() - expect(mockCline.checkpointRestore).not.toHaveBeenCalled() - expect(mockProvider.cancelTask).not.toHaveBeenCalled() - }) - - it("resolves the latest completion checkpoint in the extension host", async () => { - await webviewMessageHandler(mockProvider, { type: "completionCheckpointDiff" }) - - expect(mockCline.checkpointDiff).toHaveBeenCalledWith({ - ts: 4, - commitHash: "latest-prompt-checkpoint", - mode: "to-current", - }) + expect(fixture.providerDouble.cancelTask.mock.invocationCallOrder[0]).toBeLessThan( + fixture.taskDouble.checkpointRestore.mock.invocationCallOrder[0], + ) }) - it("does not restore when task re-initialization times out", async () => { - ;(pWaitFor as any).mockRejectedValueOnce(new Error("timed out")) - - await webviewMessageHandler(mockProvider, { type: "completionCheckpointRestore" }) - - expect(mockProvider.cancelTask).toHaveBeenCalled() - expect(mockCline.checkpointRestore).not.toHaveBeenCalled() - const vscode = await import("vscode") - expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("errors.checkpoint_timeout") + it("does not diff or restore without a latest-prompt checkpoint", async () => { + fixture.task.clineMessages = fixture.task.clineMessages.filter((message) => message.ts !== 4) + await webviewMessageHandler(fixture.provider, { type: "completionCheckpointDiff" }) + await webviewMessageHandler(fixture.provider, { type: "completionCheckpointRestore" }) + expect(fixture.taskDouble.checkpointDiff).not.toHaveBeenCalled() + expect(fixture.taskDouble.checkpointRestore).not.toHaveBeenCalled() + expect(fixture.providerDouble.cancelTask).not.toHaveBeenCalled() }) - it("shows an error when completion checkpoint restore fails", async () => { - const restoreError = new Error("restore failed") - const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}) - mockCline.checkpointRestore.mockRejectedValueOnce(restoreError) - - await webviewMessageHandler(mockProvider, { type: "completionCheckpointRestore" }) - - const vscode = await import("vscode") - expect(consoleErrorSpy).toHaveBeenCalledWith( - "[completionCheckpointRestore] checkpointRestore failed:", - restoreError, - ) - expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("errors.checkpoint_failed") - consoleErrorSpy.mockRestore() + it("does not restore when initialization times out", async () => { + vi.mocked(pWaitFor).mockRejectedValueOnce(new Error("timed out")) + await webviewMessageHandler(fixture.provider, { type: "completionCheckpointRestore" }) + expect(fixture.taskDouble.checkpointRestore).not.toHaveBeenCalled() + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("common:errors.checkpoint_timeout") }) it("does not restore when task identity changes during cancellation", async () => { - mockProvider.getCurrentTask.mockReturnValueOnce(mockCline).mockReturnValue({ - ...mockCline, - taskId: "different-task-id", - }) - - await webviewMessageHandler(mockProvider, { type: "completionCheckpointRestore" }) - - expect(mockProvider.cancelTask).toHaveBeenCalled() - expect(mockCline.checkpointRestore).not.toHaveBeenCalled() - const vscode = await import("vscode") - expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("errors.checkpoint_failed") + const replacement = createFixture() + replacement.taskDouble.taskId = "different-task-id" + fixture.providerDouble.getCurrentTask.mockReturnValueOnce(fixture.task).mockReturnValue(replacement.task) + await webviewMessageHandler(fixture.provider, { type: "completionCheckpointRestore" }) + expect(fixture.taskDouble.checkpointRestore).not.toHaveBeenCalled() + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("common:errors.checkpoint_failed") + replacement.store.clearAll() }) }) }) diff --git a/src/core/webview/checkpointRestoreHandler.ts b/src/core/webview/checkpointRestoreHandler.ts index bf276e46bc..c94c83e8f9 100644 --- a/src/core/webview/checkpointRestoreHandler.ts +++ b/src/core/webview/checkpointRestoreHandler.ts @@ -4,6 +4,8 @@ import { saveTaskMessages } from "../task-persistence" import * as vscode from "vscode" import pWaitFor from "p-wait-for" +import { t } from "../../i18n" + export interface CheckpointRestoreConfig { provider: ClineProvider currentCline: Task @@ -25,6 +27,8 @@ export interface CheckpointRestoreConfig { export async function handleCheckpointRestoreOperation(config: CheckpointRestoreConfig): Promise { const { provider, currentCline, messageTs, checkpoint, operation, editData } = config + let checkpointRestored = false + try { // For delete operations, ensure the task is properly aborted to handle any pending ask operations // This prevents "Current ask promise was ignored" errors @@ -60,6 +64,8 @@ export async function handleCheckpointRestoreOperation(config: CheckpointRestore operation, }) + checkpointRestored = true + // For delete operations, we need to save messages and reinitialize // For edit operations, the reinitialization happens automatically // and processes the pending edit @@ -78,10 +84,30 @@ export async function handleCheckpointRestoreOperation(config: CheckpointRestore // For edit operations, the task cancellation in checkpointRestore // will trigger reinitialization, which will process pendingEditAfterRestore } catch (error) { + if (operation === "edit" && !checkpointRestored) { + provider.clearPendingEditOperation(`task-${currentCline.taskId}`) + } console.error(`Error in checkpoint restore (${operation}):`, error) - vscode.window.showErrorMessage( - `Error during checkpoint restore: ${error instanceof Error ? error.message : String(error)}`, + void vscode.window.showErrorMessage( + t("common:errors.checkpoint_restore_failed", { + error: error instanceof Error ? error.message : String(error), + }), ) + if (operation === "delete" && !checkpointRestored && currentCline.abort) { + try { + const restart = t("common:checkpoints.restart") + const response = await vscode.window.showWarningMessage( + t("common:checkpoints.restart_after_restore_failure"), + restart, + ) + if (response === restart && provider.getCurrentTask() === currentCline) { + const { historyItem } = await provider.getTaskWithId(currentCline.taskId) + await provider.createTaskWithHistoryItem(historyItem) + } + } catch (restartError) { + console.error("Failed to restart task after checkpoint restore failure:", restartError) + } + } throw error } } diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index bd7c589958..41a268314e 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -1795,8 +1795,13 @@ export const webviewMessageHandler = async ( try { await provider.getCurrentTask()?.checkpointRestore(result.data) - } catch { - vscode.window.showErrorMessage(t("common:errors.checkpoint_failed")) + } catch (error) { + console.error("[checkpointRestore] checkpointRestore failed:", error) + void vscode.window.showErrorMessage( + t("common:errors.checkpoint_restore_failed", { + error: error instanceof Error ? error.message : String(error), + }), + ) } } @@ -1846,7 +1851,11 @@ export const webviewMessageHandler = async ( }) } catch (error) { console.error("[completionCheckpointRestore] checkpointRestore failed:", error) - vscode.window.showErrorMessage(t("common:errors.checkpoint_failed")) + void vscode.window.showErrorMessage( + t("common:errors.checkpoint_restore_failed", { + error: error instanceof Error ? error.message : String(error), + }), + ) } } diff --git a/src/eslint-suppressions.json b/src/eslint-suppressions.json index 8a9e1fa4df..466067eeaf 100644 --- a/src/eslint-suppressions.json +++ b/src/eslint-suppressions.json @@ -591,7 +591,7 @@ }, "core/checkpoints/__tests__/checkpoint.test.ts": { "@typescript-eslint/no-explicit-any": { - "count": 5 + "count": 3 } }, "core/checkpoints/index.ts": { @@ -1034,11 +1034,6 @@ "count": 12 } }, - "core/webview/__tests__/checkpointRestoreHandler.spec.ts": { - "@typescript-eslint/no-explicit-any": { - "count": 3 - } - }, "core/webview/__tests__/diagnosticsHandler.spec.ts": { "@typescript-eslint/no-explicit-any": { "count": 7 @@ -1059,11 +1054,6 @@ "count": 4 } }, - "core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts": { - "@typescript-eslint/no-explicit-any": { - "count": 5 - } - }, "core/webview/__tests__/webviewMessageHandler.cloudAuth.spec.ts": { "@typescript-eslint/no-explicit-any": { "count": 3 diff --git a/src/i18n/__tests__/checkpoint-translations.spec.ts b/src/i18n/__tests__/checkpoint-translations.spec.ts new file mode 100644 index 0000000000..281e73826a --- /dev/null +++ b/src/i18n/__tests__/checkpoint-translations.spec.ts @@ -0,0 +1,74 @@ +import fs from "fs/promises" +import path from "path" +import { createInstance } from "i18next" + +const localesDirectory = path.resolve(__dirname, "../locales") +const locales = [ + "ca", + "de", + "en", + "es", + "fr", + "hi", + "id", + "it", + "ja", + "ko", + "nl", + "pl", + "pt-BR", + "ru", + "tr", + "vi", + "zh-CN", + "zh-TW", +] + +interface CheckpointTranslations { + errors: { checkpoint_restore_failed: string; checkpoint_missing: string } + checkpoints: { restart_after_restore_failure: string; restart: string } +} + +async function readTranslations(locale: string): Promise { + return JSON.parse(await fs.readFile(path.join(localesDirectory, locale, "common.json"), "utf8")) +} + +describe("checkpoint translations", () => { + it.each(locales)("translates checkpoint errors and restart consent in %s", async (locale) => { + const translations = await readTranslations(locale) + const english = await readTranslations("en") + const translator = createInstance() + await translator.init({ + lng: locale, + fallbackLng: false, + resources: { [locale]: { common: translations } }, + interpolation: { escapeValue: false }, + }) + + const messages = [ + [ + "errors.checkpoint_restore_failed", + translations.errors.checkpoint_restore_failed, + english.errors.checkpoint_restore_failed, + ], + ["errors.checkpoint_missing", translations.errors.checkpoint_missing, english.errors.checkpoint_missing], + [ + "checkpoints.restart_after_restore_failure", + translations.checkpoints.restart_after_restore_failure, + english.checkpoints.restart_after_restore_failure, + ], + ["checkpoints.restart", translations.checkpoints.restart, english.checkpoints.restart], + ] + for (const [key, translated, source] of messages) { + expect(translated).toBeTruthy() + if (locale !== "en") expect(translated).not.toBe(source) + expect(translator.t(`common:${key}`)).toBe(translated) + } + expect(translations.errors.checkpoint_restore_failed).toContain("{{error}}") + expect(translations.errors.checkpoint_missing).toContain("{{commitHash}}") + expect(translator.t("common:errors.checkpoint_restore_failed", { error: "specific failure" })).toContain( + "specific failure", + ) + expect(translator.t("common:errors.checkpoint_missing", { commitHash: "abc123" })).toContain("abc123") + }) +}) diff --git a/src/i18n/locales/ca/common.json b/src/i18n/locales/ca/common.json index 579cd6b7d7..af53abe42e 100644 --- a/src/i18n/locales/ca/common.json +++ b/src/i18n/locales/ca/common.json @@ -32,6 +32,8 @@ "could_not_open_file_generic": "No s'ha pogut obrir el fitxer!", "checkpoint_timeout": "S'ha esgotat el temps en intentar restaurar el punt de control.", "checkpoint_failed": "Ha fallat la restauració del punt de control.", + "checkpoint_restore_failed": "Error durant la restauració del punt de control: {{error}}", + "checkpoint_missing": "No es pot restaurar el punt de control {{commitHash}}: aquest punt de control ja no existeix al repositori de punts de control. No s'ha modificat cap fitxer.", "git_not_installed": "Git és necessari per a la funció de punts de control. Si us plau, instal·la Git per activar els punts de control.", "checkpoint_no_first": "No hi ha un primer punt de control per comparar.", "checkpoint_no_previous": "No hi ha un punt de control anterior per comparar.", @@ -154,6 +156,10 @@ "mode_import_failed": "Ha fallat la importació del mode: {{error}}", "rooHistoryImport": "No s'ha pogut importar l'historial de Roo Code: {{error}}" }, + "checkpoints": { + "restart_after_restore_failure": "Ha fallat la restauració del punt de control i la tasca està aturada. Vols reiniciar-la?", + "restart": "Reinicia" + }, "warnings": { "no_terminal_content": "No s'ha seleccionat contingut de terminal", "missing_task_files": "Els fitxers d'aquesta tasca falten. Vols eliminar-la de la llista de tasques?", diff --git a/src/i18n/locales/de/common.json b/src/i18n/locales/de/common.json index 3d8175a77e..3db78293b3 100644 --- a/src/i18n/locales/de/common.json +++ b/src/i18n/locales/de/common.json @@ -28,6 +28,8 @@ "could_not_open_file_generic": "Datei konnte nicht geöffnet werden!", "checkpoint_timeout": "Zeitüberschreitung beim Versuch, den Checkpoint wiederherzustellen.", "checkpoint_failed": "Fehler beim Wiederherstellen des Checkpoints.", + "checkpoint_restore_failed": "Fehler beim Wiederherstellen des Checkpoints: {{error}}", + "checkpoint_missing": "Checkpoint {{commitHash}} kann nicht wiederhergestellt werden: Dieser Checkpoint existiert nicht mehr im Checkpoint-Repository. Es wurden keine Dateien geändert.", "git_not_installed": "Git ist für die Checkpoint-Funktion erforderlich. Bitte installiere Git, um Checkpoints zu aktivieren.", "checkpoint_no_first": "Kein erster Checkpoint zum Vergleich vorhanden.", "checkpoint_no_previous": "Kein vorheriger Checkpoint zum Vergleich vorhanden.", @@ -150,6 +152,10 @@ "manual_url_auth_error": "Authentifizierung fehlgeschlagen", "rooHistoryImport": "Fehler beim Importieren des Roo Code-Verlaufs: {{error}}" }, + "checkpoints": { + "restart_after_restore_failure": "Das Wiederherstellen des Checkpoints ist fehlgeschlagen und die Aufgabe wurde angehalten. Möchtest du sie neu starten?", + "restart": "Neu starten" + }, "warnings": { "no_terminal_content": "Kein Terminal-Inhalt ausgewählt", "missing_task_files": "Die Dateien dieser Aufgabe fehlen. Möchtest du sie aus der Aufgabenliste entfernen?", diff --git a/src/i18n/locales/en/common.json b/src/i18n/locales/en/common.json index a7d616028f..f3e6651a21 100644 --- a/src/i18n/locales/en/common.json +++ b/src/i18n/locales/en/common.json @@ -28,6 +28,8 @@ "could_not_open_file_generic": "Could not open file!", "checkpoint_timeout": "Timed out when attempting to restore checkpoint.", "checkpoint_failed": "Failed to restore checkpoint.", + "checkpoint_restore_failed": "Error during checkpoint restore: {{error}}", + "checkpoint_missing": "Cannot restore checkpoint {{commitHash}}: this checkpoint no longer exists in the checkpoint repository. No files were changed.", "git_not_installed": "Git is required for the checkpoints feature. Please install Git to enable checkpoints.", "checkpoint_no_first": "No first checkpoint to compare.", "checkpoint_no_previous": "No previous checkpoint to compare.", @@ -150,6 +152,10 @@ "manual_url_auth_failed": "Manual URL authentication failed", "manual_url_auth_error": "Authentication failed" }, + "checkpoints": { + "restart_after_restore_failure": "Checkpoint restore failed and the task is stopped. Do you want to restart it?", + "restart": "Restart" + }, "warnings": { "no_terminal_content": "No terminal content selected", "missing_task_files": "This task's files are missing. Would you like to remove it from the task list?", diff --git a/src/i18n/locales/es/common.json b/src/i18n/locales/es/common.json index 2eb6848f11..b24272f81a 100644 --- a/src/i18n/locales/es/common.json +++ b/src/i18n/locales/es/common.json @@ -28,6 +28,8 @@ "could_not_open_file_generic": "¡No se pudo abrir el archivo!", "checkpoint_timeout": "Se agotó el tiempo al intentar restaurar el punto de control.", "checkpoint_failed": "Error al restaurar el punto de control.", + "checkpoint_restore_failed": "Error durante la restauración del punto de control: {{error}}", + "checkpoint_missing": "No se puede restaurar el punto de control {{commitHash}}: este punto de control ya no existe en el repositorio de puntos de control. No se ha modificado ningún archivo.", "git_not_installed": "Git es necesario para la función de puntos de control. Por favor, instala Git para activar los puntos de control.", "checkpoint_no_first": "No hay primer punto de control para comparar.", "checkpoint_no_previous": "No hay punto de control anterior para comparar.", @@ -150,6 +152,10 @@ }, "rooHistoryImport": "Error al importar el historial de Roo Code: {{error}}" }, + "checkpoints": { + "restart_after_restore_failure": "La restauración del punto de control ha fallado y la tarea está detenida. ¿Quieres reiniciarla?", + "restart": "Reiniciar" + }, "warnings": { "no_terminal_content": "No hay contenido de terminal seleccionado", "missing_task_files": "Los archivos de esta tarea faltan. ¿Deseas eliminarla de la lista de tareas?", diff --git a/src/i18n/locales/fr/common.json b/src/i18n/locales/fr/common.json index b6b65a88de..a8e5c44edd 100644 --- a/src/i18n/locales/fr/common.json +++ b/src/i18n/locales/fr/common.json @@ -28,6 +28,8 @@ "could_not_open_file_generic": "Impossible d'ouvrir le fichier !", "checkpoint_timeout": "Expiration du délai lors de la tentative de rétablissement du checkpoint.", "checkpoint_failed": "Échec du rétablissement du checkpoint.", + "checkpoint_restore_failed": "Erreur lors du rétablissement du checkpoint : {{error}}", + "checkpoint_missing": "Impossible de rétablir le checkpoint {{commitHash}} : ce checkpoint n'existe plus dans le dépôt de checkpoints. Aucun fichier n'a été modifié.", "git_not_installed": "Git est requis pour la fonctionnalité des points de contrôle. Veuillez installer Git pour activer les points de contrôle.", "checkpoint_no_first": "Aucun premier point de contrôle à comparer.", "checkpoint_no_previous": "Aucun point de contrôle précédent à comparer.", @@ -150,6 +152,10 @@ }, "rooHistoryImport": "Échec de l'importation de l'historique Roo Code : {{error}}" }, + "checkpoints": { + "restart_after_restore_failure": "Le rétablissement du checkpoint a échoué et la tâche est arrêtée. Veux-tu la redémarrer ?", + "restart": "Redémarrer" + }, "warnings": { "no_terminal_content": "Aucun contenu de terminal sélectionné", "missing_task_files": "Les fichiers de cette tâche sont introuvables. Souhaitez-vous la supprimer de la liste des tâches ?", diff --git a/src/i18n/locales/hi/common.json b/src/i18n/locales/hi/common.json index f69b5adccf..ef7d1fa14d 100644 --- a/src/i18n/locales/hi/common.json +++ b/src/i18n/locales/hi/common.json @@ -28,6 +28,8 @@ "could_not_open_file_generic": "फ़ाइल नहीं खोली जा सकी!", "checkpoint_timeout": "चेकपॉइंट को पुनर्स्थापित करने का प्रयास करते समय टाइमआउट हो गया।", "checkpoint_failed": "चेकपॉइंट पुनर्स्थापित करने में विफल।", + "checkpoint_restore_failed": "चेकपॉइंट पुनर्स्थापित करते समय त्रुटि: {{error}}", + "checkpoint_missing": "चेकपॉइंट {{commitHash}} पुनर्स्थापित नहीं किया जा सकता: यह चेकपॉइंट अब चेकपॉइंट रिपॉज़िटरी में मौजूद नहीं है। किसी भी फ़ाइल में बदलाव नहीं हुआ।", "git_not_installed": "चेकपॉइंट सुविधा के लिए Git आवश्यक है। कृपया चेकपॉइंट सक्षम करने के लिए Git इंस्टॉल करें।", "checkpoint_no_first": "तुलना करने के लिए कोई पहला चेकपॉइंट नहीं है।", "checkpoint_no_previous": "तुलना करने के लिए कोई पिछला चेकपॉइंट नहीं है।", @@ -150,6 +152,10 @@ }, "rooHistoryImport": "Roo Code इतिहास आयात करने में विफल: {{error}}" }, + "checkpoints": { + "restart_after_restore_failure": "चेकपॉइंट पुनर्स्थापित नहीं हो सका और कार्य रुका हुआ है। क्या तुम इसे फिर से शुरू करना चाहते हो?", + "restart": "फिर से शुरू करो" + }, "warnings": { "no_terminal_content": "कोई टर्मिनल सामग्री चयनित नहीं", "missing_task_files": "इस टास्क की फाइलें गायब हैं। क्या आप इसे टास्क सूची से हटाना चाहते हैं?", diff --git a/src/i18n/locales/id/common.json b/src/i18n/locales/id/common.json index e401612bf3..9364c29679 100644 --- a/src/i18n/locales/id/common.json +++ b/src/i18n/locales/id/common.json @@ -28,6 +28,8 @@ "could_not_open_file_generic": "Tidak dapat membuka file!", "checkpoint_timeout": "Timeout saat mencoba memulihkan checkpoint.", "checkpoint_failed": "Gagal memulihkan checkpoint.", + "checkpoint_restore_failed": "Terjadi kesalahan saat memulihkan checkpoint: {{error}}", + "checkpoint_missing": "Tidak dapat memulihkan checkpoint {{commitHash}}: checkpoint ini sudah tidak ada di repositori checkpoint. Tidak ada file yang diubah.", "git_not_installed": "Git diperlukan untuk fitur checkpoint. Silakan instal Git untuk mengaktifkan checkpoint.", "checkpoint_no_first": "Tidak ada checkpoint pertama untuk dibandingkan.", "checkpoint_no_previous": "Tidak ada checkpoint sebelumnya untuk dibandingkan.", @@ -150,6 +152,10 @@ }, "rooHistoryImport": "Gagal mengimpor riwayat Roo Code: {{error}}" }, + "checkpoints": { + "restart_after_restore_failure": "Pemulihan checkpoint gagal dan tugas dihentikan. Mau memulainya ulang?", + "restart": "Mulai ulang" + }, "warnings": { "no_terminal_content": "Tidak ada konten terminal yang dipilih", "missing_task_files": "File tugas ini hilang. Apakah kamu ingin menghapusnya dari daftar tugas?", diff --git a/src/i18n/locales/it/common.json b/src/i18n/locales/it/common.json index aeb984940a..8c123619c5 100644 --- a/src/i18n/locales/it/common.json +++ b/src/i18n/locales/it/common.json @@ -28,6 +28,8 @@ "could_not_open_file_generic": "Impossibile aprire il file!", "checkpoint_timeout": "Timeout durante il tentativo di ripristinare il checkpoint.", "checkpoint_failed": "Impossibile ripristinare il checkpoint.", + "checkpoint_restore_failed": "Errore durante il ripristino del checkpoint: {{error}}", + "checkpoint_missing": "Impossibile ripristinare il checkpoint {{commitHash}}: questo checkpoint non esiste più nel repository dei checkpoint. Nessun file è stato modificato.", "git_not_installed": "Git è richiesto per la funzione di checkpoint. Per favore, installa Git per abilitare i checkpoint.", "checkpoint_no_first": "Nessun primo checkpoint da confrontare.", "checkpoint_no_previous": "Nessun checkpoint precedente da confrontare.", @@ -150,6 +152,10 @@ }, "rooHistoryImport": "Impossibile importare la cronologia di Roo Code: {{error}}" }, + "checkpoints": { + "restart_after_restore_failure": "Il ripristino del checkpoint non è riuscito e l'attività è ferma. Vuoi riavviarla?", + "restart": "Riavvia" + }, "warnings": { "no_terminal_content": "Nessun contenuto del terminale selezionato", "missing_task_files": "I file di questa attività sono mancanti. Vuoi rimuoverla dall'elenco delle attività?", diff --git a/src/i18n/locales/ja/common.json b/src/i18n/locales/ja/common.json index a7ec0c4de3..ba4ea444b9 100644 --- a/src/i18n/locales/ja/common.json +++ b/src/i18n/locales/ja/common.json @@ -28,6 +28,8 @@ "could_not_open_file_generic": "ファイルを開けませんでした！", "checkpoint_timeout": "チェックポイントの復元を試みる際にタイムアウトしました。", "checkpoint_failed": "チェックポイントの復元に失敗しました。", + "checkpoint_restore_failed": "チェックポイントの復元中にエラーが発生しました: {{error}}", + "checkpoint_missing": "チェックポイント {{commitHash}} を復元できません。このチェックポイントはチェックポイントリポジトリに存在しなくなっています。ファイルは変更されていません。", "git_not_installed": "チェックポイント機能にはGitが必要です。チェックポイントを有効にするにはGitをインストールしてください。", "checkpoint_no_first": "比較する最初のチェックポイントがありません。", "checkpoint_no_previous": "比較する前のチェックポイントがありません。", @@ -150,6 +152,10 @@ }, "rooHistoryImport": "Roo Codeの履歴のインポートに失敗しました: {{error}}" }, + "checkpoints": { + "restart_after_restore_failure": "チェックポイントの復元に失敗し、タスクが停止しています。再開する？", + "restart": "再開" + }, "warnings": { "no_terminal_content": "選択されたターミナルコンテンツがありません", "missing_task_files": "このタスクのファイルが見つかりません。タスクリストから削除しますか？", diff --git a/src/i18n/locales/ko/common.json b/src/i18n/locales/ko/common.json index 906652ac14..9c374134d0 100644 --- a/src/i18n/locales/ko/common.json +++ b/src/i18n/locales/ko/common.json @@ -28,6 +28,8 @@ "could_not_open_file_generic": "파일을 열 수 없습니다!", "checkpoint_timeout": "체크포인트 복원을 시도하는 중 시간 초과되었습니다.", "checkpoint_failed": "체크포인트 복원에 실패했습니다.", + "checkpoint_restore_failed": "체크포인트 복원 중 오류가 발생했어: {{error}}", + "checkpoint_missing": "체크포인트 {{commitHash}}을(를) 복원할 수 없어. 이 체크포인트는 체크포인트 저장소에 더 이상 존재하지 않아. 변경된 파일은 없어.", "git_not_installed": "체크포인트 기능을 사용하려면 Git이 필요합니다. 체크포인트를 활성화하려면 Git을 설치하세요.", "checkpoint_no_first": "비교할 첫 번째 체크포인트가 없습니다.", "checkpoint_no_previous": "비교할 이전 체크포인트가 없습니다.", @@ -150,6 +152,10 @@ }, "rooHistoryImport": "Roo Code 기록 가져오기 실패: {{error}}" }, + "checkpoints": { + "restart_after_restore_failure": "체크포인트 복원에 실패해서 작업이 멈췄어. 다시 시작할까?", + "restart": "다시 시작" + }, "warnings": { "no_terminal_content": "선택된 터미널 내용이 없습니다", "missing_task_files": "이 작업의 파일이 누락되었습니다. 작업 목록에서 제거하시겠습니까?", diff --git a/src/i18n/locales/nl/common.json b/src/i18n/locales/nl/common.json index 34cdbbbe89..0387b513b0 100644 --- a/src/i18n/locales/nl/common.json +++ b/src/i18n/locales/nl/common.json @@ -28,6 +28,8 @@ "could_not_open_file_generic": "Kon bestand niet openen!", "checkpoint_timeout": "Time-out bij het herstellen van checkpoint.", "checkpoint_failed": "Herstellen van checkpoint mislukt.", + "checkpoint_restore_failed": "Fout bij het herstellen van checkpoint: {{error}}", + "checkpoint_missing": "Checkpoint {{commitHash}} kan niet worden hersteld: dit checkpoint bestaat niet meer in de checkpoint-repository. Er zijn geen bestanden gewijzigd.", "git_not_installed": "Git is vereist voor de checkpoint-functie. Installeer Git om checkpoints in te schakelen.", "checkpoint_no_first": "Geen eerste checkpoint om mee te vergelijken.", "checkpoint_no_previous": "Geen vorig checkpoint om mee te vergelijken.", @@ -150,6 +152,10 @@ }, "rooHistoryImport": "Importeren van Roo Code-geschiedenis mislukt: {{error}}" }, + "checkpoints": { + "restart_after_restore_failure": "Het herstellen van het checkpoint is mislukt en de taak is gestopt. Wil je deze opnieuw starten?", + "restart": "Opnieuw starten" + }, "warnings": { "no_terminal_content": "Geen terminalinhoud geselecteerd", "missing_task_files": "De bestanden van deze taak ontbreken. Wil je deze uit de takenlijst verwijderen?", diff --git a/src/i18n/locales/pl/common.json b/src/i18n/locales/pl/common.json index 2a743fc009..71fed8da2c 100644 --- a/src/i18n/locales/pl/common.json +++ b/src/i18n/locales/pl/common.json @@ -28,6 +28,8 @@ "could_not_open_file_generic": "Nie można otworzyć pliku!", "checkpoint_timeout": "Upłynął limit czasu podczas próby przywrócenia punktu kontrolnego.", "checkpoint_failed": "Nie udało się przywrócić punktu kontrolnego.", + "checkpoint_restore_failed": "Błąd podczas przywracania punktu kontrolnego: {{error}}", + "checkpoint_missing": "Nie można przywrócić punktu kontrolnego {{commitHash}}: ten punkt kontrolny nie istnieje już w repozytorium punktów kontrolnych. Żadne pliki nie zostały zmienione.", "git_not_installed": "Funkcja punktów kontrolnych wymaga oprogramowania Git. Zainstaluj Git, aby włączyć punkty kontrolne.", "checkpoint_no_first": "Brak pierwszego punktu kontrolnego do porównania.", "checkpoint_no_previous": "Brak poprzedniego punktu kontrolnego do porównania.", @@ -150,6 +152,10 @@ }, "rooHistoryImport": "Nie udało się zaimportować historii Roo Code: {{error}}" }, + "checkpoints": { + "restart_after_restore_failure": "Przywracanie punktu kontrolnego nie powiodło się i zadanie jest zatrzymane. Chcesz je uruchomić ponownie?", + "restart": "Uruchom ponownie" + }, "warnings": { "no_terminal_content": "Nie wybrano zawartości terminala", "missing_task_files": "Pliki tego zadania są brakujące. Czy chcesz usunąć je z listy zadań?", diff --git a/src/i18n/locales/pt-BR/common.json b/src/i18n/locales/pt-BR/common.json index eaf1cae06b..a5346c0d1e 100644 --- a/src/i18n/locales/pt-BR/common.json +++ b/src/i18n/locales/pt-BR/common.json @@ -32,6 +32,8 @@ "could_not_open_file_generic": "Não foi possível abrir o arquivo!", "checkpoint_timeout": "Tempo esgotado ao tentar restaurar o ponto de verificação.", "checkpoint_failed": "Falha ao restaurar o ponto de verificação.", + "checkpoint_restore_failed": "Erro durante a restauração do ponto de verificação: {{error}}", + "checkpoint_missing": "Não é possível restaurar o ponto de verificação {{commitHash}}: este ponto de verificação não existe mais no repositório de pontos de verificação. Nenhum arquivo foi alterado.", "git_not_installed": "O Git é necessário para o recurso de checkpoints. Por favor, instale o Git para habilitar os checkpoints.", "checkpoint_no_first": "Nenhum primeiro ponto de verificação para comparar.", "checkpoint_no_previous": "Nenhum ponto de verificação anterior para comparar.", @@ -154,6 +156,10 @@ }, "rooHistoryImport": "Falha ao importar o histórico do Roo Code: {{error}}" }, + "checkpoints": { + "restart_after_restore_failure": "A restauração do ponto de verificação falhou e a tarefa está parada. Quer reiniciá-la?", + "restart": "Reiniciar" + }, "warnings": { "no_terminal_content": "Nenhum conteúdo do terminal selecionado", "missing_task_files": "Os arquivos desta tarefa estão faltando. Deseja removê-la da lista de tarefas?", diff --git a/src/i18n/locales/ru/common.json b/src/i18n/locales/ru/common.json index 6f48365ec1..91b5a6a55c 100644 --- a/src/i18n/locales/ru/common.json +++ b/src/i18n/locales/ru/common.json @@ -28,6 +28,8 @@ "could_not_open_file_generic": "Не удалось открыть файл!", "checkpoint_timeout": "Превышено время ожидания при попытке восстановления контрольной точки.", "checkpoint_failed": "Не удалось восстановить контрольную точку.", + "checkpoint_restore_failed": "Ошибка при восстановлении контрольной точки: {{error}}", + "checkpoint_missing": "Невозможно восстановить контрольную точку {{commitHash}}: этой контрольной точки больше нет в репозитории контрольных точек. Ни один файл не был изменён.", "git_not_installed": "Для функции контрольных точек требуется Git. Пожалуйста, установите Git, чтобы включить контрольные точки.", "checkpoint_no_first": "Нет первой контрольной точки для сравнения.", "checkpoint_no_previous": "Нет предыдущей контрольной точки для сравнения.", @@ -150,6 +152,10 @@ }, "rooHistoryImport": "Ошибка импорта истории Roo Code: {{error}}" }, + "checkpoints": { + "restart_after_restore_failure": "Не удалось восстановить контрольную точку, и задача остановлена. Хочешь перезапустить её?", + "restart": "Перезапустить" + }, "warnings": { "no_terminal_content": "Не выбрано содержимое терминала", "missing_task_files": "Файлы этой задачи отсутствуют. Хотите удалить её из списка задач?", diff --git a/src/i18n/locales/tr/common.json b/src/i18n/locales/tr/common.json index b5c543c405..4418457713 100644 --- a/src/i18n/locales/tr/common.json +++ b/src/i18n/locales/tr/common.json @@ -28,6 +28,8 @@ "could_not_open_file_generic": "Dosya açılamadı!", "checkpoint_timeout": "Kontrol noktasını geri yüklemeye çalışırken zaman aşımına uğradı.", "checkpoint_failed": "Kontrol noktası geri yüklenemedi.", + "checkpoint_restore_failed": "Kontrol noktası geri yüklenirken hata oluştu: {{error}}", + "checkpoint_missing": "{{commitHash}} kontrol noktası geri yüklenemiyor: bu kontrol noktası artık kontrol noktası deposunda mevcut değil. Hiçbir dosya değiştirilmedi.", "git_not_installed": "Kontrol noktaları özelliği için Git gereklidir. Kontrol noktalarını etkinleştirmek için lütfen Git'i yükleyin.", "checkpoint_no_first": "Karşılaştırılacak ilk kontrol noktası yok.", "checkpoint_no_previous": "Karşılaştırılacak önceki kontrol noktası yok.", @@ -150,6 +152,10 @@ }, "rooHistoryImport": "Roo Code geçmişi içe aktarılamadı: {{error}}" }, + "checkpoints": { + "restart_after_restore_failure": "Kontrol noktası geri yüklenemedi ve görev durduruldu. Yeniden başlatmak ister misin?", + "restart": "Yeniden başlat" + }, "warnings": { "no_terminal_content": "Seçili terminal içeriği yok", "missing_task_files": "Bu görevin dosyaları eksik. Görev listesinden kaldırmak istiyor musunuz?", diff --git a/src/i18n/locales/vi/common.json b/src/i18n/locales/vi/common.json index cb2ed8e9b8..85f0e34502 100644 --- a/src/i18n/locales/vi/common.json +++ b/src/i18n/locales/vi/common.json @@ -28,6 +28,8 @@ "could_not_open_file_generic": "Không thể mở tệp!", "checkpoint_timeout": "Đã hết thời gian khi cố gắng khôi phục điểm kiểm tra.", "checkpoint_failed": "Không thể khôi phục điểm kiểm tra.", + "checkpoint_restore_failed": "Lỗi khi khôi phục điểm kiểm tra: {{error}}", + "checkpoint_missing": "Không thể khôi phục điểm kiểm tra {{commitHash}}: điểm kiểm tra này không còn tồn tại trong kho lưu trữ điểm kiểm tra. Không có tệp nào bị thay đổi.", "git_not_installed": "Yêu cầu Git cho tính năng điểm kiểm tra. Vui lòng cài đặt Git để bật điểm kiểm tra.", "checkpoint_no_first": "Không có điểm kiểm tra đầu tiên để so sánh.", "checkpoint_no_previous": "Không có điểm kiểm tra trước đó để so sánh.", @@ -150,6 +152,10 @@ }, "rooHistoryImport": "Không thể nhập lịch sử Roo Code: {{error}}" }, + "checkpoints": { + "restart_after_restore_failure": "Khôi phục điểm kiểm tra thất bại và tác vụ đã dừng. Bạn muốn khởi động lại tác vụ không?", + "restart": "Khởi động lại" + }, "warnings": { "no_terminal_content": "Không có nội dung terminal được chọn", "missing_task_files": "Các tệp của nhiệm vụ này bị thiếu. Bạn có muốn xóa nó khỏi danh sách nhiệm vụ không?", diff --git a/src/i18n/locales/zh-CN/common.json b/src/i18n/locales/zh-CN/common.json index 08cfe736b5..b96d65e5b3 100644 --- a/src/i18n/locales/zh-CN/common.json +++ b/src/i18n/locales/zh-CN/common.json @@ -33,6 +33,8 @@ "could_not_open_file_generic": "无法打开文件！", "checkpoint_timeout": "尝试恢复检查点时超时。", "checkpoint_failed": "恢复检查点失败。", + "checkpoint_restore_failed": "恢复存档点时出错：{{error}}", + "checkpoint_missing": "无法恢复存档点 {{commitHash}}：此存档点已不存在于存档点仓库中。未更改任何文件。", "git_not_installed": "检查点功能需要 Git。请安装 Git 以启用检查点。", "checkpoint_no_first": "没有第一个存档点可供比较。", "checkpoint_no_previous": "没有上一个存档点可供比较。", @@ -155,6 +157,10 @@ }, "rooHistoryImport": "导入 Roo Code 历史记录失败：{{error}}" }, + "checkpoints": { + "restart_after_restore_failure": "存档点恢复失败，任务已停止。要重新启动任务吗？", + "restart": "重新启动" + }, "warnings": { "no_terminal_content": "没有选择终端内容", "missing_task_files": "此任务的文件丢失。您想从任务列表中删除它吗？", diff --git a/src/i18n/locales/zh-TW/common.json b/src/i18n/locales/zh-TW/common.json index 92fdbaeb96..859cdad32f 100644 --- a/src/i18n/locales/zh-TW/common.json +++ b/src/i18n/locales/zh-TW/common.json @@ -28,6 +28,8 @@ "could_not_open_file_generic": "無法開啟檔案！", "checkpoint_timeout": "嘗試恢復檢查點時超時。", "checkpoint_failed": "恢復檢查點失敗。", + "checkpoint_restore_failed": "還原存檔點時發生錯誤：{{error}}", + "checkpoint_missing": "無法還原存檔點 {{commitHash}}：此存檔點已不存在於存檔點儲存庫中。未變更任何檔案。", "git_not_installed": "存檔點功能需要 Git。請安裝 Git 以啟用存檔點。", "checkpoint_no_first": "沒有第一個存檔點可供比較。", "checkpoint_no_previous": "沒有上一個存檔點可供比較。", @@ -150,6 +152,10 @@ }, "rooHistoryImport": "匯入 Roo Code 歷史記錄失敗：{{error}}" }, + "checkpoints": { + "restart_after_restore_failure": "存檔點還原失敗，工作已停止。要重新啟動工作嗎？", + "restart": "重新啟動" + }, "warnings": { "no_terminal_content": "沒有選擇終端機內容", "missing_task_files": "此工作的檔案遺失。您想從工作列表中刪除它嗎？", diff --git a/src/services/checkpoints/ShadowCheckpointService.ts b/src/services/checkpoints/ShadowCheckpointService.ts index 3b00def9a0..408157d805 100644 --- a/src/services/checkpoints/ShadowCheckpointService.ts +++ b/src/services/checkpoints/ShadowCheckpointService.ts @@ -402,9 +402,7 @@ export abstract class ShadowCheckpointService extends EventEmitter { } if (!(await this.commitExists(commitHash))) { - throw new Error( - `Cannot restore checkpoint ${commitHash}: this checkpoint no longer exists in the checkpoint repository. No files were changed.`, - ) + throw new Error(t("common:errors.checkpoint_missing", { commitHash })) } const start = Date.now() @@ -424,7 +422,10 @@ export abstract class ShadowCheckpointService extends EventEmitter { } catch (e) { const error = e instanceof Error ? e : new Error(String(e)) this.log(`[${this.constructor.name}#restoreCheckpoint] failed to restore checkpoint: ${error.message}`) - this.emit("error", { type: "error", error }) + // An unhandled error event throws its payload instead of the original restore error. + if (this.listenerCount("error") > 0) { + this.emit("error", { type: "error", error }) + } throw error } } diff --git a/src/services/checkpoints/__tests__/ShadowCheckpointService.spec.ts b/src/services/checkpoints/__tests__/ShadowCheckpointService.spec.ts index 7e2dafc9bc..1da478900c 100644 --- a/src/services/checkpoints/__tests__/ShadowCheckpointService.spec.ts +++ b/src/services/checkpoints/__tests__/ShadowCheckpointService.spec.ts @@ -10,6 +10,8 @@ import { simpleGit, SimpleGit } from "simple-git" import { fileExistsAtPath } from "../../../utils/fs" import * as fileSearch from "../../../services/search/file-search" +import * as i18n from "../../../i18n" + import { RepoPerTaskCheckpointService } from "../RepoPerTaskCheckpointService" import { BLOCKED_ENV_KEYS } from "../ShadowCheckpointService" @@ -335,9 +337,13 @@ describe.each([[RepoPerTaskCheckpointService, "RepoPerTaskCheckpointService"]])( service.on("restore", restoreHandler) const checkpointsBeforeRestore = service.getCheckpoints() const missingCommitHash = "0".repeat(40) - const expectedMessage = `Cannot restore checkpoint ${missingCommitHash}: this checkpoint no longer exists in the checkpoint repository. No files were changed.` + const translateSpy = vi.spyOn(i18n, "t").mockReturnValueOnce("Localized missing checkpoint") + const expectedMessage = "Localized missing checkpoint" await expect(service.restoreCheckpoint(missingCommitHash)).rejects.toThrow(expectedMessage) + expect(translateSpy).toHaveBeenCalledWith("common:errors.checkpoint_missing", { + commitHash: missingCommitHash, + }) expect(await fs.readFile(untrackedFile, "utf-8")).toBe("Unsaved user work") expect(await fs.readFile(testFile, "utf-8")).toBe("Uncommitted edit") @@ -349,6 +355,44 @@ describe.each([[RepoPerTaskCheckpointService, "RepoPerTaskCheckpointService"]])( expect(restoreHandler).not.toHaveBeenCalled() }) + it.each(["missing", "tree", "blob"] as const)( + "preserves the original error without an error listener for a %s object", + async (objectType) => { + await fs.writeFile(testFile, "Checkpointed content") + const commit = await service.saveCheckpoint("Checkpoint") + const git = service["git"]! + const objectHash = + objectType === "missing" + ? "0".repeat(40) + : ( + await git.raw([ + "rev-parse", + `${commit!.commit}${objectType === "tree" ? "^{tree}" : ":test.txt"}`, + ]) + ).trim() + const checkpointsBeforeRestore = service.getCheckpoints() + const untrackedFile = path.join(service.workspaceDir, "precious-untracked.txt") + await fs.writeFile(untrackedFile, "Unsaved user work") + await fs.writeFile(testFile, "Uncommitted edit") + const cleanSpy = vi.spyOn(git, "clean") + const resetSpy = vi.spyOn(git, "reset") + const restoreHandler = vi.fn() + service.on("restore", restoreHandler) + expect(service.listenerCount("error")).toBe(0) + + await expect(service.restoreCheckpoint(objectHash)).rejects.toEqual( + new Error(i18n.t("common:errors.checkpoint_missing", { commitHash: objectHash })), + ) + + expect(await fs.readFile(untrackedFile, "utf-8")).toBe("Unsaved user work") + expect(await fs.readFile(testFile, "utf-8")).toBe("Uncommitted edit") + expect(service.getCheckpoints()).toEqual(checkpointsBeforeRestore) + expect(cleanSpy).not.toHaveBeenCalled() + expect(resetSpy).not.toHaveBeenCalled() + expect(restoreHandler).not.toHaveBeenCalled() + }, + ) + it("still restores normally to a valid commit", async () => { await fs.writeFile(testFile, "Checkpointed content") const commit = await service.saveCheckpoint("Valid checkpoint") @@ -387,7 +431,7 @@ describe.each([[RepoPerTaskCheckpointService, "RepoPerTaskCheckpointService"]])( await fs.writeFile(untrackedFile, "Unsaved user work") await expect(service.restoreCheckpoint(savedHash)).rejects.toThrow( - `Cannot restore checkpoint ${savedHash}: this checkpoint no longer exists`, + i18n.t("common:errors.checkpoint_missing", { commitHash: savedHash }), ) expect(await fs.readFile(untrackedFile, "utf-8")).toBe("Unsaved user work") }) @@ -399,10 +443,11 @@ describe.each([[RepoPerTaskCheckpointService, "RepoPerTaskCheckpointService"]])( await fs.writeFile(untrackedFile, "Unsaved user work") const git = service["git"]! - vitest.spyOn(git, "raw").mockRejectedValueOnce(new Error("fatal: unable to read index")) + const gitError = new Error("fatal: unable to read index") + vitest.spyOn(git, "raw").mockRejectedValueOnce(gitError) const cleanSpy = vitest.spyOn(git, "clean") - await expect(service.restoreCheckpoint(commit!.commit)).rejects.toThrow("fatal: unable to read index") + await expect(service.restoreCheckpoint(commit!.commit)).rejects.toBe(gitError) expect(cleanSpy).not.toHaveBeenCalled() expect(await fs.readFile(untrackedFile, "utf-8")).toBe("Unsaved user work") vitest.restoreAllMocks() From 8f29a9a149631dc7a27e977b997abeae02876d7d Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Sat, 31 Oct 2026 23:51:28 -0700 Subject: [PATCH 04/15] fix: scope failed-edit cleanup to its own staged edit and recheck task inside restart queue --- src/core/webview/ClineProvider.ts | 43 ++++++++-- src/core/webview/PendingEditOperationStore.ts | 13 +-- .../PendingEditOperationStore.spec.ts | 10 +++ .../checkpointRestoreHandler.spec.ts | 81 ++++++++++++++++--- .../webviewMessageHandler.checkpoint.spec.ts | 19 +++-- src/core/webview/checkpointRestoreHandler.ts | 12 +-- 6 files changed, 142 insertions(+), 36 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 10da3ec642..af0449b234 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -796,8 +796,8 @@ export class ClineProvider /** * Sets a pending edit operation with automatic timeout cleanup */ - public setPendingEditOperation(operationId: string, editData: PendingEditOperationInput): void { - this.pendingEditOperations.set(operationId, editData) + public setPendingEditOperation(operationId: string, editData: PendingEditOperationInput): object { + return this.pendingEditOperations.set(operationId, editData) } /** @@ -810,8 +810,8 @@ export class ClineProvider /** * Clears a specific pending edit operation */ - public clearPendingEditOperation(operationId: string): boolean { - return this.pendingEditOperations.clear(operationId) + public clearPendingEditOperation(operationId: string, owner?: object): boolean { + return this.pendingEditOperations.clear(operationId, owner) } /** @@ -1265,11 +1265,38 @@ export class ClineProvider // transition so both callers cannot observe the same previous registry // state and schedule distinct Task instances for one history item. // Fail forward so one rejected restoration does not poison the queue. + return ClineProvider.prototype.enqueueHistoryTaskTransition.call< + ClineProvider, + [() => Promise], + Promise + >(this, () => ClineProvider.prototype.createTaskWithHistoryItemUnlocked.call(this, historyItem, options)) + } + + /** + * Rehydrates `task` from history only if it is still current when the queued + * step runs, so a navigation queued earlier is never evicted. + */ + public restartTaskIfCurrent(task: Task): Promise { + return ClineProvider.prototype.enqueueHistoryTaskTransition.call< + ClineProvider, + [() => Promise], + Promise + >(this, async () => { + if (this.getCurrentTask() !== task) { + return false + } + const { historyItem } = await this.getTaskWithId(task.taskId) + if (this.getCurrentTask() !== task) { + return false + } + await ClineProvider.prototype.createTaskWithHistoryItemUnlocked.call(this, historyItem) + return true + }) + } + + private enqueueHistoryTaskTransition(transition: () => Promise): Promise { const previous = this.historyTaskCreationQueue ?? Promise.resolve() - const run = previous.then( - () => ClineProvider.prototype.createTaskWithHistoryItemUnlocked.call(this, historyItem, options), - () => ClineProvider.prototype.createTaskWithHistoryItemUnlocked.call(this, historyItem, options), - ) + const run = previous.then(transition, transition) this.historyTaskCreationQueue = run.then( () => {}, () => {}, diff --git a/src/core/webview/PendingEditOperationStore.ts b/src/core/webview/PendingEditOperationStore.ts index 21d545f135..dc0701131e 100644 --- a/src/core/webview/PendingEditOperationStore.ts +++ b/src/core/webview/PendingEditOperationStore.ts @@ -19,7 +19,8 @@ export class PendingEditOperationStore { private readonly log: (message: string) => void, ) {} - set(operationId: string, editData: PendingEditOperationInput): void { + /** Returns an opaque owner token; pass it to `clear` to remove only this entry. */ + set(operationId: string, editData: PendingEditOperationInput): object { this.clear(operationId) const timeoutId = setTimeout(() => { @@ -27,14 +28,16 @@ export class PendingEditOperationStore { this.log(`[PendingEditOperationStore.set] Automatically cleared stale pending operation: ${operationId}`) }, this.timeoutMs) - this.operations.set(operationId, { + const operation: PendingEditOperation = { ...editData, images: editData.images ? [...editData.images] : undefined, timeoutId, createdAt: Date.now(), - }) + } + this.operations.set(operationId, operation) this.log(`[PendingEditOperationStore.set] Set pending operation: ${operationId}`) + return operation } get(operationId: string): PendingEditOperationView | undefined { @@ -52,9 +55,9 @@ export class PendingEditOperationStore { } } - clear(operationId: string): boolean { + clear(operationId: string, owner?: object): boolean { const operation = this.operations.get(operationId) - if (!operation) { + if (!operation || (owner !== undefined && operation !== owner)) { return false } diff --git a/src/core/webview/__tests__/PendingEditOperationStore.spec.ts b/src/core/webview/__tests__/PendingEditOperationStore.spec.ts index a35a04415d..e8290523a8 100644 --- a/src/core/webview/__tests__/PendingEditOperationStore.spec.ts +++ b/src/core/webview/__tests__/PendingEditOperationStore.spec.ts @@ -48,6 +48,16 @@ describe("PendingEditOperationStore", () => { expect(store.get("op")).toBeUndefined() }) + it("owner-scoped clear removes only the entry that owner staged", () => { + const staleOwner = store.set("op", editData) + const currentOwner = store.set("op", { ...editData, editedContent: "newer" }) + + expect(store.clear("op", staleOwner)).toBe(false) + expect(store.get("op")?.editedContent).toBe("newer") + expect(store.clear("op", currentOwner)).toBe(true) + expect(store.get("op")).toBeUndefined() + }) + it("hides timer metadata from get results", () => { store.set("op", editData) diff --git a/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts b/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts index 9371cb21f9..d63218b4a6 100644 --- a/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts +++ b/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts @@ -88,6 +88,7 @@ function createFixture() { apiConversationHistory: [], }), createTaskWithHistoryItem: vi.fn().mockResolvedValue(task), + restartTaskIfCurrent: vi.fn(), setPendingEditOperation: vi.fn(), clearPendingEditOperation: vi.fn(), getPendingEditOperation: ClineProvider.prototype["getPendingEditOperation"], @@ -99,8 +100,11 @@ function createFixture() { providerDouble.setPendingEditOperation.mockImplementation((operationId, editData) => ClineProvider.prototype.setPendingEditOperation.call(provider, operationId, editData), ) - providerDouble.clearPendingEditOperation.mockImplementation((operationId) => - ClineProvider.prototype.clearPendingEditOperation.call(provider, operationId), + providerDouble.clearPendingEditOperation.mockImplementation((operationId, owner) => + ClineProvider.prototype.clearPendingEditOperation.call(provider, operationId, owner), + ) + providerDouble.restartTaskIfCurrent.mockImplementation((restartTask) => + ClineProvider.prototype.restartTaskIfCurrent.call(provider, restartTask), ) const config: CheckpointRestoreConfig = { provider, @@ -214,23 +218,82 @@ describe("checkpointRestoreHandler", () => { expect(showWarningMessage).not.toHaveBeenCalled() expect(fixture.providerDouble.clearPendingEditOperation).toHaveBeenCalledWith( `task-${fixture.task.taskId}`, + expect.any(Object), ) } }, ) - it("does not restart a different current task when the user accepts", async () => { + it("restarts the failed task when it is still current and the user accepts", async () => { fixture.taskDouble.checkpointRestore.mockRejectedValueOnce(new Error("Missing commit")) - showWarningMessage.mockImplementationOnce(async () => { - fixture.providerDouble.getCurrentTask.mockReturnValue(undefined) - return "common:checkpoints.restart" - }) + showWarningMessage.mockResolvedValueOnce("common:checkpoints.restart") await expect(handleCheckpointRestoreOperation(fixture.config)).rejects.toThrow("Missing commit") - expect(fixture.providerDouble.getTaskWithId).not.toHaveBeenCalled() - expect(fixture.providerDouble.createTaskWithHistoryItem).not.toHaveBeenCalled() + expect(fixture.providerDouble.getTaskWithId).toHaveBeenCalledExactlyOnceWith(fixture.task.taskId) + expect(Task).toHaveBeenCalledOnce() + }) + + it.each([ + { + name: "before the restart is queued", + setup: (current: ReturnType) => + showWarningMessage.mockImplementationOnce(async () => { + current.providerDouble.getCurrentTask.mockReturnValue(undefined) + return "common:checkpoints.restart" + }), + loadsHistory: false, + }, + { + name: "while history is loading", + setup: (current: ReturnType) => { + showWarningMessage.mockResolvedValueOnce("common:checkpoints.restart") + current.providerDouble.getTaskWithId.mockImplementationOnce(async () => { + current.providerDouble.getCurrentTask.mockReturnValue(undefined) + return { historyItem: current.historyItem } as Awaited> + }) + }, + loadsHistory: true, + }, + { + name: "by a navigation queued first", + setup: (current: ReturnType) => + showWarningMessage.mockImplementationOnce(async () => { + void ClineProvider.prototype["enqueueHistoryTaskTransition"].call(current.provider, async () => { + current.providerDouble.getCurrentTask.mockReturnValue(undefined) + }) + return "common:checkpoints.restart" + }), + loadsHistory: false, + }, + ])("does not restart when another task becomes current $name", async ({ setup, loadsHistory }) => { + fixture.taskDouble.checkpointRestore.mockRejectedValueOnce(new Error("Missing commit")) + setup(fixture) + await expect(handleCheckpointRestoreOperation(fixture.config)).rejects.toThrow("Missing commit") + expect(fixture.providerDouble.getTaskWithId).toHaveBeenCalledTimes(loadsHistory ? 1 : 0) + expect(Task).not.toHaveBeenCalled() expect(saveTaskMessages).not.toHaveBeenCalled() }) + it("a failed older edit keeps a newer edit staged during its restore", async () => { + const operationId = `task-${fixture.task.taskId}` + fixture.taskDouble.checkpointRestore.mockImplementationOnce(async () => { + fixture.provider.setPendingEditOperation(operationId, { + messageTs: 3, + editedContent: "Newer edit", + messageIndex: 1, + apiConversationHistoryIndex: 0, + }) + throw new Error("Missing commit") + }) + await expect( + handleCheckpointRestoreOperation({ + ...fixture.config, + operation: "edit", + editData: { editedContent: "Older edit", apiConversationHistoryIndex: 0 }, + }), + ).rejects.toThrow("Missing commit") + expect(fixture.store.get(operationId)?.editedContent).toBe("Newer edit") + }) + it("does not apply a failed pending edit on later real provider rehydration", async () => { vi.useFakeTimers() const messages = structuredClone(fixture.task.clineMessages) diff --git a/src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts index bfdb941adc..b15a7c9f48 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts @@ -99,6 +99,7 @@ function createFixture() { apiConversationHistory: [], }), createTaskWithHistoryItem: vi.fn().mockResolvedValue(task), + restartTaskIfCurrent: vi.fn().mockResolvedValue(true), setPendingEditOperation: vi.fn(), clearPendingEditOperation: vi.fn(), cancelTask: vi.fn(), @@ -109,8 +110,8 @@ function createFixture() { providerDouble.setPendingEditOperation.mockImplementation((operationId, editData) => ClineProvider.prototype.setPendingEditOperation.call(provider, operationId, editData), ) - providerDouble.clearPendingEditOperation.mockImplementation((operationId) => - ClineProvider.prototype.clearPendingEditOperation.call(provider, operationId), + providerDouble.clearPendingEditOperation.mockImplementation((operationId, owner) => + ClineProvider.prototype.clearPendingEditOperation.call(provider, operationId, owner), ) return { task, taskDouble, provider, providerDouble, historyItem, store } } @@ -139,7 +140,7 @@ describe("webviewMessageHandler - checkpoint operations", () => { } it.each([true, false])("failed delete restarts only after accepting the prompt: %s", async (accept) => { - const { task, taskDouble, provider, providerDouble, historyItem } = fixture + const { task, taskDouble, provider, providerDouble } = fixture const messages = structuredClone(task.clineMessages) const history = structuredClone(task.apiConversationHistory) const restoreError = new Error("Checkpoint commit is missing") @@ -190,12 +191,11 @@ describe("webviewMessageHandler - checkpoint operations", () => { answerPrompt(accept ? "common:checkpoints.restart" : undefined) await operation if (accept) { - expect(providerDouble.getTaskWithId).toHaveBeenCalledExactlyOnceWith(task.taskId) - expect(providerDouble.createTaskWithHistoryItem).toHaveBeenCalledExactlyOnceWith(historyItem) + expect(providerDouble.restartTaskIfCurrent).toHaveBeenCalledExactlyOnceWith(task) } else { - expect(providerDouble.getTaskWithId).not.toHaveBeenCalled() - expect(providerDouble.createTaskWithHistoryItem).not.toHaveBeenCalled() + expect(providerDouble.restartTaskIfCurrent).not.toHaveBeenCalled() } + expect(providerDouble.createTaskWithHistoryItem).not.toHaveBeenCalled() expectUnchanged(messages, history) expect(vscode.window.showErrorMessage).toHaveBeenCalledTimes(2) expect(vscode.window.showErrorMessage).toHaveBeenLastCalledWith( @@ -235,7 +235,10 @@ describe("webviewMessageHandler - checkpoint operations", () => { error: "Checkpoint belongs to another task", }) expect(providerDouble.getTaskWithId).not.toHaveBeenCalled() - expect(providerDouble.clearPendingEditOperation).toHaveBeenCalledExactlyOnceWith(operationId) + expect(providerDouble.clearPendingEditOperation).toHaveBeenCalledExactlyOnceWith( + operationId, + expect.any(Object), + ) expect(store.get(operationId)).toBeUndefined() expect(taskDouble.abortTask).not.toHaveBeenCalled() expect(providerDouble.createTaskWithHistoryItem).not.toHaveBeenCalled() diff --git a/src/core/webview/checkpointRestoreHandler.ts b/src/core/webview/checkpointRestoreHandler.ts index c94c83e8f9..d8b1acb4b3 100644 --- a/src/core/webview/checkpointRestoreHandler.ts +++ b/src/core/webview/checkpointRestoreHandler.ts @@ -28,6 +28,7 @@ export async function handleCheckpointRestoreOperation(config: CheckpointRestore const { provider, currentCline, messageTs, checkpoint, operation, editData } = config let checkpointRestored = false + let pendingEditOwner: object | undefined try { // For delete operations, ensure the task is properly aborted to handle any pending ask operations @@ -47,7 +48,7 @@ export async function handleCheckpointRestoreOperation(config: CheckpointRestore // For edit operations, set up pending edit data before restoration if (operation === "edit" && editData) { const operationId = `task-${currentCline.taskId}` - provider.setPendingEditOperation(operationId, { + pendingEditOwner = provider.setPendingEditOperation(operationId, { messageTs, editedContent: editData.editedContent, images: editData.images, @@ -84,8 +85,8 @@ export async function handleCheckpointRestoreOperation(config: CheckpointRestore // For edit operations, the task cancellation in checkpointRestore // will trigger reinitialization, which will process pendingEditAfterRestore } catch (error) { - if (operation === "edit" && !checkpointRestored) { - provider.clearPendingEditOperation(`task-${currentCline.taskId}`) + if (operation === "edit" && !checkpointRestored && pendingEditOwner) { + provider.clearPendingEditOperation(`task-${currentCline.taskId}`, pendingEditOwner) } console.error(`Error in checkpoint restore (${operation}):`, error) void vscode.window.showErrorMessage( @@ -100,9 +101,8 @@ export async function handleCheckpointRestoreOperation(config: CheckpointRestore t("common:checkpoints.restart_after_restore_failure"), restart, ) - if (response === restart && provider.getCurrentTask() === currentCline) { - const { historyItem } = await provider.getTaskWithId(currentCline.taskId) - await provider.createTaskWithHistoryItem(historyItem) + if (response === restart) { + await provider.restartTaskIfCurrent(currentCline) } } catch (restartError) { console.error("Failed to restart task after checkpoint restore failure:", restartError) From c90b7f347aa6c5d494e292faabb8a9803f20b59b Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Sun, 1 Nov 2026 00:39:07 -0700 Subject: [PATCH 05/15] fix: skip in-place restart replacement when another task became current --- src/core/webview/ClineProvider.ts | 6 +++++ .../checkpointRestoreHandler.spec.ts | 26 +++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index af0449b234..139c1dd2f1 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -1446,6 +1446,12 @@ export class ClineProvider // Replace the current task in-place to avoid UI flicker const oldTask = this.taskRegistry.current + // A different task may have become current during the awaits above; never abort it. + if (oldTask && oldTask.taskId !== task.taskId) { + await task.abortTask(true).catch(() => {}) + return task + } + if (oldTask) { // Abort the old task to stop running processes and mark as abandoned try { diff --git a/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts b/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts index d63218b4a6..6cdebbf1e0 100644 --- a/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts +++ b/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts @@ -94,6 +94,7 @@ function createFixture() { getPendingEditOperation: ClineProvider.prototype["getPendingEditOperation"], getState: vi.fn(async () => ({ apiConfiguration: {} })), contextProxy: { globalStorageUri: { fsPath: "/test/storage" } }, + taskScheduler: { schedule: vi.fn<(task: Task, run: () => Promise) => Promise>(async () => {}) }, } // Supplies restore/rehydration dependencies and the real store; scheduling is disabled in the rehydration test. const provider = providerDouble as unknown as ClineProvider @@ -230,6 +231,31 @@ describe("checkpointRestoreHandler", () => { await expect(handleCheckpointRestoreOperation(fixture.config)).rejects.toThrow("Missing commit") expect(fixture.providerDouble.getTaskWithId).toHaveBeenCalledExactlyOnceWith(fixture.task.taskId) expect(Task).toHaveBeenCalledOnce() + expect(fixture.providerDouble.taskScheduler.schedule).toHaveBeenCalledExactlyOnceWith( + fixture.task, + expect.any(Function), + ) + expect(console.error).not.toHaveBeenCalledWith( + "Failed to restart task after checkpoint restore failure:", + expect.anything(), + ) + }) + + it("restart does not abort a different task that became current during rehydration setup", async () => { + const newerTask = { taskId: "newer-task", abortTask: vi.fn() } + const registry = { current: fixture.task as Task | typeof newerTask, replace: vi.fn() } + Object.assign(fixture.providerDouble, { taskRegistry: registry }) + fixture.taskDouble.checkpointRestore.mockRejectedValueOnce(new Error("Missing commit")) + showWarningMessage.mockResolvedValueOnce("common:checkpoints.restart") + fixture.providerDouble.getState.mockImplementationOnce(async () => { + registry.current = newerTask + return { apiConfiguration: {} } + }) + await expect(handleCheckpointRestoreOperation(fixture.config)).rejects.toThrow("Missing commit") + expect(Task).toHaveBeenCalledOnce() + expect(newerTask.abortTask).not.toHaveBeenCalled() + expect(registry.replace).not.toHaveBeenCalled() + expect(fixture.providerDouble.taskScheduler.schedule).not.toHaveBeenCalled() }) it.each([ From 965b60ebf5ea1cc0edcedd5e94b00c9c674361d5 Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Sun, 1 Nov 2026 01:35:32 -0700 Subject: [PATCH 06/15] fix: scope history profile restore to the rehydrated task and skip replacement when no task is current --- src/core/webview/ClineProvider.ts | 38 ++++++++++++++----- .../ClineProvider.sticky-profile.spec.ts | 30 ++++++++++++++- .../checkpointRestoreHandler.spec.ts | 14 ++++--- 3 files changed, 66 insertions(+), 16 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 139c1dd2f1..7eb9d23063 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -1364,7 +1364,10 @@ export class ClineProvider const hasActualSettings = !!fullProfile.apiProvider if (hasActualSettings) { - await this.activateProviderProfile({ name: profile.name }) + await this.activateProviderProfile( + { name: profile.name }, + { expectedCurrentTaskId: historyItem.id }, + ) } else { // The task will continue with the current/default configuration. } @@ -1396,7 +1399,11 @@ export class ClineProvider if (profile.apiProvider) { await this.activateProviderProfile( { name: profile.name }, - { persistModeConfig: false, persistTaskHistory: false }, + { + persistModeConfig: false, + persistTaskHistory: false, + expectedCurrentTaskId: historyItem.id, + }, ) } } catch (error) { @@ -1446,8 +1453,8 @@ export class ClineProvider // Replace the current task in-place to avoid UI flicker const oldTask = this.taskRegistry.current - // A different task may have become current during the awaits above; never abort it. - if (oldTask && oldTask.taskId !== task.taskId) { + // The current task may have changed or been removed during the awaits above; never touch it. + if (oldTask?.taskId !== task.taskId) { await task.abortTask(true).catch(() => {}) return task } @@ -1923,11 +1930,11 @@ export class ClineProvider */ private updateTaskApiHandlerIfNeeded( providerSettings: ProviderSettings, - options: { forceRebuild?: boolean; skipCurrentTaskRebuild?: boolean } = {}, + options: { forceRebuild?: boolean; skipCurrentTaskRebuild?: boolean; expectedCurrentTaskId?: string } = {}, ): void { if (options.skipCurrentTaskRebuild) return const task = this.getCurrentTask() - if (!task) return + if (!task || (options.expectedCurrentTaskId && task.taskId !== options.expectedCurrentTaskId)) return const { forceRebuild = false } = options @@ -2169,11 +2176,11 @@ export class ClineProvider private async persistStickyProviderProfileToCurrentTask( apiConfigName: string, - options: { skipCurrentTaskRebuild?: boolean } = {}, + options: { skipCurrentTaskRebuild?: boolean; expectedCurrentTaskId?: string } = {}, ): Promise { if (options.skipCurrentTaskRebuild) return const task = this.getCurrentTask() - if (!task) { + if (!task || (options.expectedCurrentTaskId && task.taskId !== options.expectedCurrentTaskId)) { return } @@ -2203,6 +2210,8 @@ export class ClineProvider persistModeConfig?: boolean persistTaskHistory?: boolean skipCurrentTaskRebuild?: boolean + /** Rebuild and sticky-write only when this task is still current. */ + expectedCurrentTaskId?: string }, ) { return this.enqueueProviderProfileMutation((signal) => @@ -2216,6 +2225,7 @@ export class ClineProvider persistModeConfig?: boolean persistTaskHistory?: boolean skipCurrentTaskRebuild?: boolean + expectedCurrentTaskId?: string }, signal?: AbortSignal, ): Promise { @@ -2243,12 +2253,20 @@ export class ClineProvider } // Change the provider for the current task. - this.updateTaskApiHandlerIfNeeded(providerSettings, { forceRebuild: true, skipCurrentTaskRebuild }) + const expectedCurrentTaskId = options?.expectedCurrentTaskId + this.updateTaskApiHandlerIfNeeded(providerSettings, { + forceRebuild: true, + skipCurrentTaskRebuild, + expectedCurrentTaskId, + }) // Update the current task's sticky provider profile, unless this activation is // being used purely as a non-persisting restoration (e.g., reopening a task from history). if (persistTaskHistory) { - await this.persistStickyProviderProfileToCurrentTask(name, { skipCurrentTaskRebuild }) + await this.persistStickyProviderProfileToCurrentTask(name, { + skipCurrentTaskRebuild, + expectedCurrentTaskId, + }) } if (!skipCurrentTaskRebuild) { diff --git a/src/core/webview/__tests__/ClineProvider.sticky-profile.spec.ts b/src/core/webview/__tests__/ClineProvider.sticky-profile.spec.ts index 178415aa6d..1ce9e8e5a2 100644 --- a/src/core/webview/__tests__/ClineProvider.sticky-profile.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.sticky-profile.spec.ts @@ -5,6 +5,7 @@ import * as vscode from "vscode" import { makeCompositeDisposable } from "../../../test-utils/vscode" import { TelemetryService } from "@roo-code/telemetry" import { ClineProvider } from "../ClineProvider" +import type { Task } from "../../task/Task" import { ContextProxy } from "../../config/ContextProxy" import type { HistoryItem } from "@roo-code/types" import { providerIdentifiers } from "@roo-code/types/provider-identifiers" @@ -395,6 +396,33 @@ describe("ClineProvider - Sticky Provider Profile", () => { expect(mockTask.setTaskApiConfigName).toHaveBeenCalledWith("new-profile") }) + it.each([ + { expectedCurrentTaskId: "test-task-id", applies: true }, + { expectedCurrentTaskId: "other-task-id", applies: false }, + ])( + "expectedCurrentTaskId $expectedCurrentTaskId rebuilds and sticky-writes the current task: $applies", + async ({ expectedCurrentTaskId, applies }) => { + const currentTask = { + taskId: "test-task-id", + setTaskApiConfigName: vi.fn<(name: string) => void>(), + updateApiConfiguration: vi.fn<(settings: unknown) => void>(), + emit: vi.fn(), + } + // Supplies only the members that profile activation touches on the current task. + vi.spyOn(provider, "getCurrentTask").mockReturnValue(currentTask as unknown as Task) + vi.spyOn(provider.providerSettingsManager, "activateProfile").mockResolvedValue({ + name: "new-profile", + id: "new-profile-id", + apiProvider: providerIdentifiers.anthropic, + }) + + await provider.activateProviderProfile({ name: "new-profile" }, { expectedCurrentTaskId }) + + expect(currentTask.updateApiConfiguration).toHaveBeenCalledTimes(applies ? 1 : 0) + expect(currentTask.setTaskApiConfigName).toHaveBeenCalledTimes(applies ? 1 : 0) + }, + ) + it("should update task's taskApiConfigName property when switching profiles", async () => { // Create a mock task with initial profile const mockTask = { @@ -531,7 +559,7 @@ describe("ClineProvider - Sticky Provider Profile", () => { // Verify provider profile was restored via activateProviderProfile (restore-only: don't persist mode config) expect(activateProviderProfileSpy).toHaveBeenCalledWith( { name: "saved-profile" }, - { persistModeConfig: false, persistTaskHistory: false }, + { persistModeConfig: false, persistTaskHistory: false, expectedCurrentTaskId: historyItem.id }, ) }) diff --git a/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts b/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts index 6cdebbf1e0..f789f34820 100644 --- a/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts +++ b/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts @@ -76,7 +76,8 @@ function createFixture() { const store = new PendingEditOperationStore(60_000, vi.fn()) const providerDouble = { pendingEditOperations: store, - taskRegistry: { current: undefined }, + taskRegistry: { current: task as Pick | undefined, replace: vi.fn() }, + taskEventListeners: new Map void>>(), performPreparationTasks: vi.fn<(task: Task) => Promise>(), log: vi.fn<(message: string) => void>(), getCurrentTask: vi.fn(() => task), @@ -241,18 +242,21 @@ describe("checkpointRestoreHandler", () => { ) }) - it("restart does not abort a different task that became current during rehydration setup", async () => { + it.each([ + { name: "a different task became current", replacement: "newer" }, + { name: "no task remains current", replacement: "none" }, + ] as const)("restart skips replacement and scheduling when $name during setup", async ({ replacement }) => { const newerTask = { taskId: "newer-task", abortTask: vi.fn() } - const registry = { current: fixture.task as Task | typeof newerTask, replace: vi.fn() } - Object.assign(fixture.providerDouble, { taskRegistry: registry }) + const registry = fixture.providerDouble.taskRegistry fixture.taskDouble.checkpointRestore.mockRejectedValueOnce(new Error("Missing commit")) showWarningMessage.mockResolvedValueOnce("common:checkpoints.restart") fixture.providerDouble.getState.mockImplementationOnce(async () => { - registry.current = newerTask + registry.current = replacement === "newer" ? newerTask : undefined return { apiConfiguration: {} } }) await expect(handleCheckpointRestoreOperation(fixture.config)).rejects.toThrow("Missing commit") expect(Task).toHaveBeenCalledOnce() + expect(fixture.taskDouble.abortTask).toHaveBeenLastCalledWith(true) expect(newerTask.abortTask).not.toHaveBeenCalled() expect(registry.replace).not.toHaveBeenCalled() expect(fixture.providerDouble.taskScheduler.schedule).not.toHaveBeenCalled() From 0171bab4bec6b335a6130426600109ddd18df7e4 Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Sun, 1 Nov 2026 01:27:43 -0800 Subject: [PATCH 07/15] fix: skip mode-config persistence on history profile restore; tighten restart and sticky-write test assertions --- src/core/webview/ClineProvider.ts | 2 +- .../ClineProvider.sticky-profile.spec.ts | 49 +++++++++++++++++++ .../checkpointRestoreHandler.spec.ts | 5 +- 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 7eb9d23063..7a6a79322e 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -1366,7 +1366,7 @@ export class ClineProvider if (hasActualSettings) { await this.activateProviderProfile( { name: profile.name }, - { expectedCurrentTaskId: historyItem.id }, + { persistModeConfig: false, expectedCurrentTaskId: historyItem.id }, ) } else { // The task will continue with the current/default configuration. diff --git a/src/core/webview/__tests__/ClineProvider.sticky-profile.spec.ts b/src/core/webview/__tests__/ClineProvider.sticky-profile.spec.ts index 1ce9e8e5a2..ce8abbddb5 100644 --- a/src/core/webview/__tests__/ClineProvider.sticky-profile.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.sticky-profile.spec.ts @@ -410,6 +410,16 @@ describe("ClineProvider - Sticky Provider Profile", () => { } // Supplies only the members that profile activation touches on the current task. vi.spyOn(provider, "getCurrentTask").mockReturnValue(currentTask as unknown as Task) + await provider.taskHistoryStore.upsert({ + id: currentTask.taskId, + ts: Date.now(), + task: "Test task", + number: 1, + tokensIn: 0, + tokensOut: 0, + totalCost: 0, + }) + const updateTaskHistorySpy = vi.spyOn(provider, "updateTaskHistory").mockResolvedValue([]) vi.spyOn(provider.providerSettingsManager, "activateProfile").mockResolvedValue({ name: "new-profile", id: "new-profile-id", @@ -420,6 +430,12 @@ describe("ClineProvider - Sticky Provider Profile", () => { expect(currentTask.updateApiConfiguration).toHaveBeenCalledTimes(applies ? 1 : 0) expect(currentTask.setTaskApiConfigName).toHaveBeenCalledTimes(applies ? 1 : 0) + const stickyWrite = expect.objectContaining({ id: currentTask.taskId, apiConfigName: "new-profile" }) + if (applies) { + expect(updateTaskHistorySpy).toHaveBeenCalledWith(stickyWrite) + } else { + expect(updateTaskHistorySpy).not.toHaveBeenCalledWith(stickyWrite) + } }, ) @@ -658,6 +674,39 @@ describe("ClineProvider - Sticky Provider Profile", () => { expect(activateProviderProfileSpy).not.toHaveBeenCalled() }) + it("restores the mode-based profile without persisting it as a mode default or touching another task", async () => { + await provider.resolveWebviewView(mockWebviewView) + const historyItem: HistoryItem = { + id: "test-task-id", + number: 1, + ts: Date.now(), + task: "Test task", + tokensIn: 0, + tokensOut: 0, + totalCost: 0, + mode: "code", + } + const activateProviderProfileSpy = vi + .spyOn(provider, "activateProviderProfile") + .mockResolvedValue(undefined) + vi.spyOn(provider.providerSettingsManager, "getModeConfigId").mockResolvedValue("mode-config-id") + vi.spyOn(provider.providerSettingsManager, "listConfig").mockResolvedValue([ + { name: "mode-profile", id: "mode-config-id", apiProvider: providerIdentifiers.anthropic }, + ]) + vi.spyOn(provider.providerSettingsManager, "getProfile").mockResolvedValue({ + name: "mode-profile", + id: "mode-config-id", + apiProvider: providerIdentifiers.anthropic, + }) + + await provider.createTaskWithHistoryItem(historyItem) + + expect(activateProviderProfileSpy).toHaveBeenCalledWith( + { name: "mode-profile" }, + { persistModeConfig: false, expectedCurrentTaskId: historyItem.id }, + ) + }) + it("should use current profile if history item has no saved apiConfigName", async () => { await provider.resolveWebviewView(mockWebviewView) diff --git a/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts b/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts index f789f34820..7896f5b73b 100644 --- a/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts +++ b/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts @@ -247,6 +247,8 @@ describe("checkpointRestoreHandler", () => { { name: "no task remains current", replacement: "none" }, ] as const)("restart skips replacement and scheduling when $name during setup", async ({ replacement }) => { const newerTask = { taskId: "newer-task", abortTask: vi.fn() } + const constructedTask = { taskId: fixture.task.taskId, abortTask: vi.fn(async () => {}) } + constructTask.mockReturnValueOnce(constructedTask) const registry = fixture.providerDouble.taskRegistry fixture.taskDouble.checkpointRestore.mockRejectedValueOnce(new Error("Missing commit")) showWarningMessage.mockResolvedValueOnce("common:checkpoints.restart") @@ -256,7 +258,8 @@ describe("checkpointRestoreHandler", () => { }) await expect(handleCheckpointRestoreOperation(fixture.config)).rejects.toThrow("Missing commit") expect(Task).toHaveBeenCalledOnce() - expect(fixture.taskDouble.abortTask).toHaveBeenLastCalledWith(true) + expect(constructedTask.abortTask).toHaveBeenCalledExactlyOnceWith(true) + expect(fixture.taskDouble.abortTask).not.toHaveBeenCalledWith(true) expect(newerTask.abortTask).not.toHaveBeenCalled() expect(registry.replace).not.toHaveBeenCalled() expect(fixture.providerDouble.taskScheduler.schedule).not.toHaveBeenCalled() From 10d1c8d524a9b23c0a529088431ee871d09faa3a Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Sun, 1 Nov 2026 02:18:50 -0800 Subject: [PATCH 08/15] test: tighten sticky-profile assertions for mismatched task and mode-default write --- .../ClineProvider.sticky-profile.spec.ts | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/core/webview/__tests__/ClineProvider.sticky-profile.spec.ts b/src/core/webview/__tests__/ClineProvider.sticky-profile.spec.ts index ce8abbddb5..386cfe7c83 100644 --- a/src/core/webview/__tests__/ClineProvider.sticky-profile.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.sticky-profile.spec.ts @@ -434,11 +434,30 @@ describe("ClineProvider - Sticky Provider Profile", () => { if (applies) { expect(updateTaskHistorySpy).toHaveBeenCalledWith(stickyWrite) } else { - expect(updateTaskHistorySpy).not.toHaveBeenCalledWith(stickyWrite) + expect(updateTaskHistorySpy).not.toHaveBeenCalled() } }, ) + it.each([true, false])( + "writes the mode default only when persistModeConfig is %s", + async (persistModeConfig) => { + vi.spyOn(provider.providerSettingsManager, "activateProfile").mockResolvedValue({ + name: "new-profile", + id: "new-profile-id", + apiProvider: providerIdentifiers.anthropic, + }) + const setModeConfigSpy = vi.spyOn(provider.providerSettingsManager, "setModeConfig").mockResolvedValue() + + await provider.activateProviderProfile( + { name: "new-profile" }, + { persistModeConfig, expectedCurrentTaskId: "test-task-id" }, + ) + + expect(setModeConfigSpy).toHaveBeenCalledTimes(persistModeConfig ? 1 : 0) + }, + ) + it("should update task's taskApiConfigName property when switching profiles", async () => { // Create a mock task with initial profile const mockTask = { From 22cb84b8bfd0b626308424037d96b0003cdbb379 Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Sun, 1 Nov 2026 03:08:55 -0800 Subject: [PATCH 09/15] test: assert setModeConfig mode and profile id --- .../webview/__tests__/ClineProvider.sticky-profile.spec.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/core/webview/__tests__/ClineProvider.sticky-profile.spec.ts b/src/core/webview/__tests__/ClineProvider.sticky-profile.spec.ts index 386cfe7c83..11a3fafeaf 100644 --- a/src/core/webview/__tests__/ClineProvider.sticky-profile.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.sticky-profile.spec.ts @@ -448,13 +448,18 @@ describe("ClineProvider - Sticky Provider Profile", () => { apiProvider: providerIdentifiers.anthropic, }) const setModeConfigSpy = vi.spyOn(provider.providerSettingsManager, "setModeConfig").mockResolvedValue() + await provider.contextProxy.setValue("mode", "architect") await provider.activateProviderProfile( { name: "new-profile" }, { persistModeConfig, expectedCurrentTaskId: "test-task-id" }, ) - expect(setModeConfigSpy).toHaveBeenCalledTimes(persistModeConfig ? 1 : 0) + if (persistModeConfig) { + expect(setModeConfigSpy).toHaveBeenCalledExactlyOnceWith("architect", "new-profile-id") + } else { + expect(setModeConfigSpy).not.toHaveBeenCalled() + } }, ) From 86777f9eeb7a7015aa3e2fcb371b002fa9d4f42b Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Sun, 1 Nov 2026 04:02:46 -0800 Subject: [PATCH 10/15] fix: reject unrecorded or malformed checkpoint hashes before any Git call checkpointRestore and checkpointDiff now require a full 40-hex hash recorded as a checkpoint_saved message of the current task, covering delete, edit, checkpointRestore, completionCheckpointRestore and checkpointDiff. Adds tests for delete restores that succeed and then fail while saving or rehydrating. --- .../checkpoints/__tests__/checkpoint.test.ts | 74 ++++++++++++++----- src/core/checkpoints/index.ts | 24 ++++++ .../checkpointRestoreHandler.spec.ts | 31 +++++++- src/i18n/locales/ca/common.json | 1 + src/i18n/locales/de/common.json | 1 + src/i18n/locales/en/common.json | 1 + src/i18n/locales/es/common.json | 1 + src/i18n/locales/fr/common.json | 1 + src/i18n/locales/hi/common.json | 1 + src/i18n/locales/id/common.json | 1 + src/i18n/locales/it/common.json | 1 + src/i18n/locales/ja/common.json | 1 + src/i18n/locales/ko/common.json | 1 + src/i18n/locales/nl/common.json | 1 + src/i18n/locales/pl/common.json | 1 + src/i18n/locales/pt-BR/common.json | 1 + src/i18n/locales/ru/common.json | 1 + src/i18n/locales/tr/common.json | 1 + src/i18n/locales/vi/common.json | 1 + src/i18n/locales/zh-CN/common.json | 1 + src/i18n/locales/zh-TW/common.json | 1 + 21 files changed, 126 insertions(+), 21 deletions(-) diff --git a/src/core/checkpoints/__tests__/checkpoint.test.ts b/src/core/checkpoints/__tests__/checkpoint.test.ts index dcce376109..efb6ee30ae 100644 --- a/src/core/checkpoints/__tests__/checkpoint.test.ts +++ b/src/core/checkpoints/__tests__/checkpoint.test.ts @@ -1,8 +1,13 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest" import { checkpointSave, checkpointRestore, checkpointDiff, getCheckpointService } from "../index" import { MessageManager } from "../../message-manager" +import { RepoPerTaskCheckpointService } from "../../../services/checkpoints" import * as vscode from "vscode" +const RESTORE_HASH = "a".repeat(40) +const HASH_1 = "1".repeat(40) +const HASH_2 = "2".repeat(40) + // Mock vscode vi.mock("vscode", () => ({ window: { @@ -207,6 +212,7 @@ describe("Checkpoint functionality", () => { { ts: 1, say: "user", text: "Message 1" }, { ts: 2, say: "assistant", text: "Message 2" }, { ts: 3, say: "user", text: "Message 3" }, + { ts: 4, say: "checkpoint_saved", text: RESTORE_HASH }, ] mockTask.apiConversationHistory = [ { ts: 1, role: "user", content: [{ type: "text", text: "Message 1" }] }, @@ -218,12 +224,12 @@ describe("Checkpoint functionality", () => { it("should restore checkpoint for delete operation", async () => { await checkpointRestore(mockTask, { ts: 2, - commitHash: "abc123", + commitHash: RESTORE_HASH, mode: "restore", operation: "delete", }) - expect(mockCheckpointService.restoreCheckpoint).toHaveBeenCalledWith("abc123") + expect(mockCheckpointService.restoreCheckpoint).toHaveBeenCalledWith(RESTORE_HASH) expect(mockTask.overwriteApiConversationHistory).toHaveBeenCalledWith([ { ts: 1, role: "user", content: [{ type: "text", text: "Message 1" }] }, ]) @@ -234,12 +240,12 @@ describe("Checkpoint functionality", () => { it("should restore checkpoint for edit operation", async () => { await checkpointRestore(mockTask, { ts: 2, - commitHash: "abc123", + commitHash: RESTORE_HASH, mode: "restore", operation: "edit", }) - expect(mockCheckpointService.restoreCheckpoint).toHaveBeenCalledWith("abc123") + expect(mockCheckpointService.restoreCheckpoint).toHaveBeenCalledWith(RESTORE_HASH) expect(mockTask.overwriteApiConversationHistory).toHaveBeenCalledWith([ { ts: 1, role: "user", content: [{ type: "text", text: "Message 1" }] }, ]) @@ -254,11 +260,11 @@ describe("Checkpoint functionality", () => { it("should handle preview mode without modifying messages", async () => { await checkpointRestore(mockTask, { ts: 2, - commitHash: "abc123", + commitHash: RESTORE_HASH, mode: "preview", }) - expect(mockCheckpointService.restoreCheckpoint).toHaveBeenCalledWith("abc123") + expect(mockCheckpointService.restoreCheckpoint).toHaveBeenCalledWith(RESTORE_HASH) expect(mockTask.overwriteApiConversationHistory).not.toHaveBeenCalled() expect(mockTask.overwriteClineMessages).not.toHaveBeenCalled() expect(mockProvider.cancelTask).toHaveBeenCalled() @@ -267,7 +273,7 @@ describe("Checkpoint functionality", () => { it("should handle missing message gracefully", async () => { await checkpointRestore(mockTask, { ts: 999, // Non-existent timestamp - commitHash: "abc123", + commitHash: RESTORE_HASH, mode: "restore", }) @@ -285,7 +291,7 @@ describe("Checkpoint functionality", () => { await expect( checkpointRestore(mockTask, { ts: 2, - commitHash: "abc123", + commitHash: RESTORE_HASH, mode: "restore", operation, }), @@ -302,15 +308,31 @@ describe("Checkpoint functionality", () => { expect(mockProvider.log).toHaveBeenCalledWith("[checkpointRestore] failed to restore checkpoint") }, ) + + it.each([ + ["a malformed hash", "HEAD~1"], + ["an abbreviated hash", "a".repeat(7)], + ["a well-formed but unrecorded hash", "b".repeat(40)], + ["a non-string", 42], + ])("rejects %s before any Git call", async (_label, commitHash) => { + await expect( + checkpointRestore(mockTask, { ts: 2, commitHash: commitHash as string, mode: "restore" }), + ).rejects.toThrow("common:errors.checkpoint_invalid_hash") + + expect(RepoPerTaskCheckpointService.create).not.toHaveBeenCalled() + expect(mockCheckpointService.restoreCheckpoint).not.toHaveBeenCalled() + expect(mockTask.overwriteClineMessages).not.toHaveBeenCalled() + expect(mockProvider.cancelTask).not.toHaveBeenCalled() + }) }) describe("checkpointDiff", () => { beforeEach(() => { mockTask.clineMessages = [ { ts: 1, say: "user", text: "Message 1" }, - { ts: 2, say: "checkpoint_saved", text: "commit1" }, + { ts: 2, say: "checkpoint_saved", text: HASH_1 }, { ts: 3, say: "user", text: "Message 2" }, - { ts: 4, say: "checkpoint_saved", text: "commit2" }, + { ts: 4, say: "checkpoint_saved", text: HASH_2 }, ] }) @@ -325,12 +347,12 @@ describe("Checkpoint functionality", () => { await checkpointDiff(mockTask, { ts: 4, - commitHash: "commit2", + commitHash: HASH_2, mode: "to-current", }) expect(mockCheckpointService.getDiff).toHaveBeenCalledWith({ - from: "commit2", + from: HASH_2, to: undefined, }) expect(vscode.commands.executeCommand).toHaveBeenCalledWith( @@ -350,13 +372,13 @@ describe("Checkpoint functionality", () => { mockCheckpointService.getDiff.mockResolvedValue(mockChanges) await checkpointDiff(mockTask, { ts: 4, - commitHash: "commit1", + commitHash: HASH_1, mode: "checkpoint", }) expect(mockCheckpointService.getDiff).toHaveBeenCalledWith({ - from: "commit1", - to: "commit2", + from: HASH_1, + to: HASH_2, }) expect(vscode.commands.executeCommand).toHaveBeenCalledWith( "vscode.changes", @@ -376,13 +398,13 @@ describe("Checkpoint functionality", () => { await checkpointDiff(mockTask, { ts: 4, - commitHash: "commit1", + commitHash: HASH_1, mode: "checkpoint", }) expect(mockCheckpointService.getDiff).toHaveBeenCalledWith({ - from: "commit1", // Should find the next checkpoint - to: "commit2", + from: HASH_1, // Should find the next checkpoint + to: HASH_2, }) }) @@ -391,7 +413,7 @@ describe("Checkpoint functionality", () => { await checkpointDiff(mockTask, { ts: 4, - commitHash: "commit2", + commitHash: HASH_2, mode: "to-current", }) @@ -399,12 +421,24 @@ describe("Checkpoint functionality", () => { expect(vscode.commands.executeCommand).not.toHaveBeenCalled() }) + it.each([ + ["a malformed hash", "HEAD~1"], + ["a well-formed but unrecorded hash", "b".repeat(40)], + ["a non-string", 42], + ])("rejects %s before any Git call", async (_label, commitHash) => { + await checkpointDiff(mockTask, { commitHash: commitHash as string, mode: "to-current" }) + + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("common:errors.checkpoint_invalid_hash") + expect(mockCheckpointService.getDiff).not.toHaveBeenCalled() + expect(mockTask.enableCheckpoints).toBe(true) + }) + it("should disable checkpoints on error", async () => { mockCheckpointService.getDiff.mockRejectedValue(new Error("Diff failed")) await checkpointDiff(mockTask, { ts: 4, - commitHash: "commit2", + commitHash: HASH_2, mode: "to-current", }) diff --git a/src/core/checkpoints/index.ts b/src/core/checkpoints/index.ts index 64c50f4a15..63e908fbff 100644 --- a/src/core/checkpoints/index.ts +++ b/src/core/checkpoints/index.ts @@ -227,6 +227,21 @@ export async function checkpointSave(task: Task, force = false, suppressMessage }) } +// The shadow repo is created with `git init` defaults (SHA-1) and stores full hashes. +const FULL_COMMIT_HASH = /^[0-9a-f]{40}$/ + +// The hash arrives from the webview, so it must name a checkpoint this task recorded before reaching Git. +function assertRecordedCheckpoint(task: Task, commitHash: unknown): asserts commitHash is string { + const isRecorded = + typeof commitHash === "string" && + FULL_COMMIT_HASH.test(commitHash) && + task.clineMessages.some(({ say, text }) => say === "checkpoint_saved" && text === commitHash) + + if (!isRecorded) { + throw new Error(t("common:errors.checkpoint_invalid_hash")) + } +} + export type CheckpointRestoreOptions = { ts: number commitHash: string @@ -238,6 +253,8 @@ export async function checkpointRestore( task: Task, { ts, commitHash, mode, operation = "delete" }: CheckpointRestoreOptions, ) { + assertRecordedCheckpoint(task, commitHash) + const service = await getCheckpointService(task) if (!service) { @@ -315,6 +332,13 @@ export type CheckpointDiffOptions = { } export async function checkpointDiff(task: Task, { commitHash, mode }: CheckpointDiffOptions) { + try { + assertRecordedCheckpoint(task, commitHash) + } catch (error) { + void vscode.window.showErrorMessage(error instanceof Error ? error.message : String(error)) + return + } + const service = await getCheckpointService(task) if (!service) { diff --git a/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts b/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts index 7896f5b73b..e736722263 100644 --- a/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts +++ b/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts @@ -116,7 +116,8 @@ function createFixture() { checkpoint: { hash: "abc123" }, operation: "delete", } - return { task, taskDouble, provider, providerDouble, historyItem, store, config } + const failure = new Error("Disk full") + return { task, taskDouble, provider, providerDouble, historyItem, store, config, failure } } describe("checkpointRestoreHandler", () => { @@ -226,6 +227,34 @@ describe("checkpointRestoreHandler", () => { }, ) + it.each([ + { + step: "saving messages", + fail: (f: typeof fixture) => vi.mocked(saveTaskMessages).mockRejectedValueOnce(f.failure), + }, + { + step: "loading history", + fail: (f: typeof fixture) => f.providerDouble.getTaskWithId.mockRejectedValueOnce(f.failure), + }, + { + step: "rehydrating the task", + fail: (f: typeof fixture) => f.providerDouble.createTaskWithHistoryItem.mockRejectedValueOnce(f.failure), + }, + ])( + "delete restore that succeeds then fails while $step surfaces the error without a restart prompt", + async ({ fail }) => { + fail(fixture) + await expect(handleCheckpointRestoreOperation(fixture.config)).rejects.toBe(fixture.failure) + expect(fixture.taskDouble.checkpointRestore).toHaveBeenCalledOnce() + expect(vscode.window.showErrorMessage).toHaveBeenCalledExactlyOnceWith( + "common:errors.checkpoint_restore_failed: Disk full", + ) + expect(console.error).toHaveBeenCalledWith("Error in checkpoint restore (delete):", fixture.failure) + expect(showWarningMessage).not.toHaveBeenCalled() + expect(fixture.providerDouble.restartTaskIfCurrent).not.toHaveBeenCalled() + }, + ) + it("restarts the failed task when it is still current and the user accepts", async () => { fixture.taskDouble.checkpointRestore.mockRejectedValueOnce(new Error("Missing commit")) showWarningMessage.mockResolvedValueOnce("common:checkpoints.restart") diff --git a/src/i18n/locales/ca/common.json b/src/i18n/locales/ca/common.json index af53abe42e..234ccc05c6 100644 --- a/src/i18n/locales/ca/common.json +++ b/src/i18n/locales/ca/common.json @@ -34,6 +34,7 @@ "checkpoint_failed": "Ha fallat la restauració del punt de control.", "checkpoint_restore_failed": "Error durant la restauració del punt de control: {{error}}", "checkpoint_missing": "No es pot restaurar el punt de control {{commitHash}}: aquest punt de control ja no existeix al repositori de punts de control. No s'ha modificat cap fitxer.", + "checkpoint_invalid_hash": "No es pot restaurar ni comparar el punt de control: la referència no correspon a cap punt de control vàlid d'aquesta tasca. No s'ha modificat cap fitxer.", "git_not_installed": "Git és necessari per a la funció de punts de control. Si us plau, instal·la Git per activar els punts de control.", "checkpoint_no_first": "No hi ha un primer punt de control per comparar.", "checkpoint_no_previous": "No hi ha un punt de control anterior per comparar.", diff --git a/src/i18n/locales/de/common.json b/src/i18n/locales/de/common.json index 3db78293b3..1ba612ff1f 100644 --- a/src/i18n/locales/de/common.json +++ b/src/i18n/locales/de/common.json @@ -30,6 +30,7 @@ "checkpoint_failed": "Fehler beim Wiederherstellen des Checkpoints.", "checkpoint_restore_failed": "Fehler beim Wiederherstellen des Checkpoints: {{error}}", "checkpoint_missing": "Checkpoint {{commitHash}} kann nicht wiederhergestellt werden: Dieser Checkpoint existiert nicht mehr im Checkpoint-Repository. Es wurden keine Dateien geändert.", + "checkpoint_invalid_hash": "Checkpoint kann nicht wiederhergestellt oder verglichen werden: Die Referenz ist kein gültiger Checkpoint dieser Aufgabe. Es wurden keine Dateien geändert.", "git_not_installed": "Git ist für die Checkpoint-Funktion erforderlich. Bitte installiere Git, um Checkpoints zu aktivieren.", "checkpoint_no_first": "Kein erster Checkpoint zum Vergleich vorhanden.", "checkpoint_no_previous": "Kein vorheriger Checkpoint zum Vergleich vorhanden.", diff --git a/src/i18n/locales/en/common.json b/src/i18n/locales/en/common.json index f3e6651a21..9e87bb2d22 100644 --- a/src/i18n/locales/en/common.json +++ b/src/i18n/locales/en/common.json @@ -30,6 +30,7 @@ "checkpoint_failed": "Failed to restore checkpoint.", "checkpoint_restore_failed": "Error during checkpoint restore: {{error}}", "checkpoint_missing": "Cannot restore checkpoint {{commitHash}}: this checkpoint no longer exists in the checkpoint repository. No files were changed.", + "checkpoint_invalid_hash": "Cannot restore or compare checkpoint: the checkpoint reference is not a valid checkpoint of this task. No files were changed.", "git_not_installed": "Git is required for the checkpoints feature. Please install Git to enable checkpoints.", "checkpoint_no_first": "No first checkpoint to compare.", "checkpoint_no_previous": "No previous checkpoint to compare.", diff --git a/src/i18n/locales/es/common.json b/src/i18n/locales/es/common.json index b24272f81a..c704840a24 100644 --- a/src/i18n/locales/es/common.json +++ b/src/i18n/locales/es/common.json @@ -30,6 +30,7 @@ "checkpoint_failed": "Error al restaurar el punto de control.", "checkpoint_restore_failed": "Error durante la restauración del punto de control: {{error}}", "checkpoint_missing": "No se puede restaurar el punto de control {{commitHash}}: este punto de control ya no existe en el repositorio de puntos de control. No se ha modificado ningún archivo.", + "checkpoint_invalid_hash": "No se puede restaurar ni comparar el punto de control: la referencia no es un punto de control válido de esta tarea. No se ha modificado ningún archivo.", "git_not_installed": "Git es necesario para la función de puntos de control. Por favor, instala Git para activar los puntos de control.", "checkpoint_no_first": "No hay primer punto de control para comparar.", "checkpoint_no_previous": "No hay punto de control anterior para comparar.", diff --git a/src/i18n/locales/fr/common.json b/src/i18n/locales/fr/common.json index a8e5c44edd..73b6cf6974 100644 --- a/src/i18n/locales/fr/common.json +++ b/src/i18n/locales/fr/common.json @@ -30,6 +30,7 @@ "checkpoint_failed": "Échec du rétablissement du checkpoint.", "checkpoint_restore_failed": "Erreur lors du rétablissement du checkpoint : {{error}}", "checkpoint_missing": "Impossible de rétablir le checkpoint {{commitHash}} : ce checkpoint n'existe plus dans le dépôt de checkpoints. Aucun fichier n'a été modifié.", + "checkpoint_invalid_hash": "Impossible de rétablir ou de comparer le checkpoint : la référence ne correspond à aucun checkpoint valide de cette tâche. Aucun fichier n'a été modifié.", "git_not_installed": "Git est requis pour la fonctionnalité des points de contrôle. Veuillez installer Git pour activer les points de contrôle.", "checkpoint_no_first": "Aucun premier point de contrôle à comparer.", "checkpoint_no_previous": "Aucun point de contrôle précédent à comparer.", diff --git a/src/i18n/locales/hi/common.json b/src/i18n/locales/hi/common.json index ef7d1fa14d..5ad882ddad 100644 --- a/src/i18n/locales/hi/common.json +++ b/src/i18n/locales/hi/common.json @@ -30,6 +30,7 @@ "checkpoint_failed": "चेकपॉइंट पुनर्स्थापित करने में विफल।", "checkpoint_restore_failed": "चेकपॉइंट पुनर्स्थापित करते समय त्रुटि: {{error}}", "checkpoint_missing": "चेकपॉइंट {{commitHash}} पुनर्स्थापित नहीं किया जा सकता: यह चेकपॉइंट अब चेकपॉइंट रिपॉज़िटरी में मौजूद नहीं है। किसी भी फ़ाइल में बदलाव नहीं हुआ।", + "checkpoint_invalid_hash": "चेकपॉइंट को पुनर्स्थापित या तुलना नहीं किया जा सकता: यह संदर्भ इस कार्य का मान्य चेकपॉइंट नहीं है। किसी भी फ़ाइल में बदलाव नहीं हुआ।", "git_not_installed": "चेकपॉइंट सुविधा के लिए Git आवश्यक है। कृपया चेकपॉइंट सक्षम करने के लिए Git इंस्टॉल करें।", "checkpoint_no_first": "तुलना करने के लिए कोई पहला चेकपॉइंट नहीं है।", "checkpoint_no_previous": "तुलना करने के लिए कोई पिछला चेकपॉइंट नहीं है।", diff --git a/src/i18n/locales/id/common.json b/src/i18n/locales/id/common.json index 9364c29679..00a6514843 100644 --- a/src/i18n/locales/id/common.json +++ b/src/i18n/locales/id/common.json @@ -30,6 +30,7 @@ "checkpoint_failed": "Gagal memulihkan checkpoint.", "checkpoint_restore_failed": "Terjadi kesalahan saat memulihkan checkpoint: {{error}}", "checkpoint_missing": "Tidak dapat memulihkan checkpoint {{commitHash}}: checkpoint ini sudah tidak ada di repositori checkpoint. Tidak ada file yang diubah.", + "checkpoint_invalid_hash": "Tidak dapat memulihkan atau membandingkan checkpoint: referensi ini bukan checkpoint yang valid untuk tugas ini. Tidak ada file yang diubah.", "git_not_installed": "Git diperlukan untuk fitur checkpoint. Silakan instal Git untuk mengaktifkan checkpoint.", "checkpoint_no_first": "Tidak ada checkpoint pertama untuk dibandingkan.", "checkpoint_no_previous": "Tidak ada checkpoint sebelumnya untuk dibandingkan.", diff --git a/src/i18n/locales/it/common.json b/src/i18n/locales/it/common.json index 8c123619c5..aee485321f 100644 --- a/src/i18n/locales/it/common.json +++ b/src/i18n/locales/it/common.json @@ -30,6 +30,7 @@ "checkpoint_failed": "Impossibile ripristinare il checkpoint.", "checkpoint_restore_failed": "Errore durante il ripristino del checkpoint: {{error}}", "checkpoint_missing": "Impossibile ripristinare il checkpoint {{commitHash}}: questo checkpoint non esiste più nel repository dei checkpoint. Nessun file è stato modificato.", + "checkpoint_invalid_hash": "Impossibile ripristinare o confrontare il checkpoint: il riferimento non è un checkpoint valido di questa attività. Nessun file è stato modificato.", "git_not_installed": "Git è richiesto per la funzione di checkpoint. Per favore, installa Git per abilitare i checkpoint.", "checkpoint_no_first": "Nessun primo checkpoint da confrontare.", "checkpoint_no_previous": "Nessun checkpoint precedente da confrontare.", diff --git a/src/i18n/locales/ja/common.json b/src/i18n/locales/ja/common.json index ba4ea444b9..c9bc4d5b50 100644 --- a/src/i18n/locales/ja/common.json +++ b/src/i18n/locales/ja/common.json @@ -30,6 +30,7 @@ "checkpoint_failed": "チェックポイントの復元に失敗しました。", "checkpoint_restore_failed": "チェックポイントの復元中にエラーが発生しました: {{error}}", "checkpoint_missing": "チェックポイント {{commitHash}} を復元できません。このチェックポイントはチェックポイントリポジトリに存在しなくなっています。ファイルは変更されていません。", + "checkpoint_invalid_hash": "チェックポイントを復元または比較できません。この参照はこのタスクの有効なチェックポイントではありません。ファイルは変更されていません。", "git_not_installed": "チェックポイント機能にはGitが必要です。チェックポイントを有効にするにはGitをインストールしてください。", "checkpoint_no_first": "比較する最初のチェックポイントがありません。", "checkpoint_no_previous": "比較する前のチェックポイントがありません。", diff --git a/src/i18n/locales/ko/common.json b/src/i18n/locales/ko/common.json index 9c374134d0..b9b7fef312 100644 --- a/src/i18n/locales/ko/common.json +++ b/src/i18n/locales/ko/common.json @@ -30,6 +30,7 @@ "checkpoint_failed": "체크포인트 복원에 실패했습니다.", "checkpoint_restore_failed": "체크포인트 복원 중 오류가 발생했어: {{error}}", "checkpoint_missing": "체크포인트 {{commitHash}}을(를) 복원할 수 없어. 이 체크포인트는 체크포인트 저장소에 더 이상 존재하지 않아. 변경된 파일은 없어.", + "checkpoint_invalid_hash": "체크포인트를 복원하거나 비교할 수 없어. 이 참조는 이 작업의 유효한 체크포인트가 아니야. 변경된 파일은 없어.", "git_not_installed": "체크포인트 기능을 사용하려면 Git이 필요합니다. 체크포인트를 활성화하려면 Git을 설치하세요.", "checkpoint_no_first": "비교할 첫 번째 체크포인트가 없습니다.", "checkpoint_no_previous": "비교할 이전 체크포인트가 없습니다.", diff --git a/src/i18n/locales/nl/common.json b/src/i18n/locales/nl/common.json index 0387b513b0..8533aefee9 100644 --- a/src/i18n/locales/nl/common.json +++ b/src/i18n/locales/nl/common.json @@ -30,6 +30,7 @@ "checkpoint_failed": "Herstellen van checkpoint mislukt.", "checkpoint_restore_failed": "Fout bij het herstellen van checkpoint: {{error}}", "checkpoint_missing": "Checkpoint {{commitHash}} kan niet worden hersteld: dit checkpoint bestaat niet meer in de checkpoint-repository. Er zijn geen bestanden gewijzigd.", + "checkpoint_invalid_hash": "Checkpoint kan niet worden hersteld of vergeleken: de verwijzing is geen geldig checkpoint van deze taak. Er zijn geen bestanden gewijzigd.", "git_not_installed": "Git is vereist voor de checkpoint-functie. Installeer Git om checkpoints in te schakelen.", "checkpoint_no_first": "Geen eerste checkpoint om mee te vergelijken.", "checkpoint_no_previous": "Geen vorig checkpoint om mee te vergelijken.", diff --git a/src/i18n/locales/pl/common.json b/src/i18n/locales/pl/common.json index 71fed8da2c..16bf6ceb2e 100644 --- a/src/i18n/locales/pl/common.json +++ b/src/i18n/locales/pl/common.json @@ -30,6 +30,7 @@ "checkpoint_failed": "Nie udało się przywrócić punktu kontrolnego.", "checkpoint_restore_failed": "Błąd podczas przywracania punktu kontrolnego: {{error}}", "checkpoint_missing": "Nie można przywrócić punktu kontrolnego {{commitHash}}: ten punkt kontrolny nie istnieje już w repozytorium punktów kontrolnych. Żadne pliki nie zostały zmienione.", + "checkpoint_invalid_hash": "Nie można przywrócić ani porównać punktu kontrolnego: odwołanie nie jest prawidłowym punktem kontrolnym tego zadania. Żadne pliki nie zostały zmienione.", "git_not_installed": "Funkcja punktów kontrolnych wymaga oprogramowania Git. Zainstaluj Git, aby włączyć punkty kontrolne.", "checkpoint_no_first": "Brak pierwszego punktu kontrolnego do porównania.", "checkpoint_no_previous": "Brak poprzedniego punktu kontrolnego do porównania.", diff --git a/src/i18n/locales/pt-BR/common.json b/src/i18n/locales/pt-BR/common.json index a5346c0d1e..9020fda929 100644 --- a/src/i18n/locales/pt-BR/common.json +++ b/src/i18n/locales/pt-BR/common.json @@ -34,6 +34,7 @@ "checkpoint_failed": "Falha ao restaurar o ponto de verificação.", "checkpoint_restore_failed": "Erro durante a restauração do ponto de verificação: {{error}}", "checkpoint_missing": "Não é possível restaurar o ponto de verificação {{commitHash}}: este ponto de verificação não existe mais no repositório de pontos de verificação. Nenhum arquivo foi alterado.", + "checkpoint_invalid_hash": "Não é possível restaurar ou comparar o ponto de verificação: a referência não é um ponto de verificação válido desta tarefa. Nenhum arquivo foi alterado.", "git_not_installed": "O Git é necessário para o recurso de checkpoints. Por favor, instale o Git para habilitar os checkpoints.", "checkpoint_no_first": "Nenhum primeiro ponto de verificação para comparar.", "checkpoint_no_previous": "Nenhum ponto de verificação anterior para comparar.", diff --git a/src/i18n/locales/ru/common.json b/src/i18n/locales/ru/common.json index 91b5a6a55c..6f80c30766 100644 --- a/src/i18n/locales/ru/common.json +++ b/src/i18n/locales/ru/common.json @@ -30,6 +30,7 @@ "checkpoint_failed": "Не удалось восстановить контрольную точку.", "checkpoint_restore_failed": "Ошибка при восстановлении контрольной точки: {{error}}", "checkpoint_missing": "Невозможно восстановить контрольную точку {{commitHash}}: этой контрольной точки больше нет в репозитории контрольных точек. Ни один файл не был изменён.", + "checkpoint_invalid_hash": "Невозможно восстановить или сравнить контрольную точку: ссылка не является действительной контрольной точкой этой задачи. Ни один файл не был изменён.", "git_not_installed": "Для функции контрольных точек требуется Git. Пожалуйста, установите Git, чтобы включить контрольные точки.", "checkpoint_no_first": "Нет первой контрольной точки для сравнения.", "checkpoint_no_previous": "Нет предыдущей контрольной точки для сравнения.", diff --git a/src/i18n/locales/tr/common.json b/src/i18n/locales/tr/common.json index 4418457713..639e42fd19 100644 --- a/src/i18n/locales/tr/common.json +++ b/src/i18n/locales/tr/common.json @@ -30,6 +30,7 @@ "checkpoint_failed": "Kontrol noktası geri yüklenemedi.", "checkpoint_restore_failed": "Kontrol noktası geri yüklenirken hata oluştu: {{error}}", "checkpoint_missing": "{{commitHash}} kontrol noktası geri yüklenemiyor: bu kontrol noktası artık kontrol noktası deposunda mevcut değil. Hiçbir dosya değiştirilmedi.", + "checkpoint_invalid_hash": "Kontrol noktası geri yüklenemiyor veya karşılaştırılamıyor: bu başvuru, bu görevin geçerli bir kontrol noktası değil. Hiçbir dosya değiştirilmedi.", "git_not_installed": "Kontrol noktaları özelliği için Git gereklidir. Kontrol noktalarını etkinleştirmek için lütfen Git'i yükleyin.", "checkpoint_no_first": "Karşılaştırılacak ilk kontrol noktası yok.", "checkpoint_no_previous": "Karşılaştırılacak önceki kontrol noktası yok.", diff --git a/src/i18n/locales/vi/common.json b/src/i18n/locales/vi/common.json index 85f0e34502..7b68a47582 100644 --- a/src/i18n/locales/vi/common.json +++ b/src/i18n/locales/vi/common.json @@ -30,6 +30,7 @@ "checkpoint_failed": "Không thể khôi phục điểm kiểm tra.", "checkpoint_restore_failed": "Lỗi khi khôi phục điểm kiểm tra: {{error}}", "checkpoint_missing": "Không thể khôi phục điểm kiểm tra {{commitHash}}: điểm kiểm tra này không còn tồn tại trong kho lưu trữ điểm kiểm tra. Không có tệp nào bị thay đổi.", + "checkpoint_invalid_hash": "Không thể khôi phục hoặc so sánh điểm kiểm tra: tham chiếu này không phải là điểm kiểm tra hợp lệ của tác vụ này. Không có tệp nào bị thay đổi.", "git_not_installed": "Yêu cầu Git cho tính năng điểm kiểm tra. Vui lòng cài đặt Git để bật điểm kiểm tra.", "checkpoint_no_first": "Không có điểm kiểm tra đầu tiên để so sánh.", "checkpoint_no_previous": "Không có điểm kiểm tra trước đó để so sánh.", diff --git a/src/i18n/locales/zh-CN/common.json b/src/i18n/locales/zh-CN/common.json index b96d65e5b3..c2c1487ea8 100644 --- a/src/i18n/locales/zh-CN/common.json +++ b/src/i18n/locales/zh-CN/common.json @@ -35,6 +35,7 @@ "checkpoint_failed": "恢复检查点失败。", "checkpoint_restore_failed": "恢复存档点时出错：{{error}}", "checkpoint_missing": "无法恢复存档点 {{commitHash}}：此存档点已不存在于存档点仓库中。未更改任何文件。", + "checkpoint_invalid_hash": "无法恢复或比较存档点：该引用不是此任务的有效存档点。未更改任何文件。", "git_not_installed": "检查点功能需要 Git。请安装 Git 以启用检查点。", "checkpoint_no_first": "没有第一个存档点可供比较。", "checkpoint_no_previous": "没有上一个存档点可供比较。", diff --git a/src/i18n/locales/zh-TW/common.json b/src/i18n/locales/zh-TW/common.json index 859cdad32f..2cace13e8b 100644 --- a/src/i18n/locales/zh-TW/common.json +++ b/src/i18n/locales/zh-TW/common.json @@ -30,6 +30,7 @@ "checkpoint_failed": "恢復檢查點失敗。", "checkpoint_restore_failed": "還原存檔點時發生錯誤：{{error}}", "checkpoint_missing": "無法還原存檔點 {{commitHash}}：此存檔點已不存在於存檔點儲存庫中。未變更任何檔案。", + "checkpoint_invalid_hash": "無法還原或比較存檔點：此參照不是此工作的有效存檔點。未變更任何檔案。", "git_not_installed": "存檔點功能需要 Git。請安裝 Git 以啟用存檔點。", "checkpoint_no_first": "沒有第一個存檔點可供比較。", "checkpoint_no_previous": "沒有上一個存檔點可供比較。", From 584f720ae58450810101fbbb32912c1f57fd0cdd Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Sun, 1 Nov 2026 04:49:49 -0800 Subject: [PATCH 11/15] fix: accept SHA-256 checkpoint hashes in restore validation --- src/core/checkpoints/__tests__/checkpoint.test.ts | 10 ++++++++++ src/core/checkpoints/index.ts | 4 ++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/core/checkpoints/__tests__/checkpoint.test.ts b/src/core/checkpoints/__tests__/checkpoint.test.ts index efb6ee30ae..b7e9931de6 100644 --- a/src/core/checkpoints/__tests__/checkpoint.test.ts +++ b/src/core/checkpoints/__tests__/checkpoint.test.ts @@ -309,9 +309,19 @@ describe("Checkpoint functionality", () => { }, ) + it("accepts a recorded SHA-256 checkpoint hash", async () => { + const sha256Hash = "c".repeat(64) + mockTask.clineMessages.push({ ts: 5, say: "checkpoint_saved", text: sha256Hash }) + + await checkpointRestore(mockTask, { ts: 2, commitHash: sha256Hash, mode: "preview" }) + + expect(mockCheckpointService.restoreCheckpoint).toHaveBeenCalledWith(sha256Hash) + }) + it.each([ ["a malformed hash", "HEAD~1"], ["an abbreviated hash", "a".repeat(7)], + ["a hash between SHA-1 and SHA-256 length", "a".repeat(41)], ["a well-formed but unrecorded hash", "b".repeat(40)], ["a non-string", 42], ])("rejects %s before any Git call", async (_label, commitHash) => { diff --git a/src/core/checkpoints/index.ts b/src/core/checkpoints/index.ts index 63e908fbff..77a67e5c91 100644 --- a/src/core/checkpoints/index.ts +++ b/src/core/checkpoints/index.ts @@ -227,8 +227,8 @@ export async function checkpointSave(task: Task, force = false, suppressMessage }) } -// The shadow repo is created with `git init` defaults (SHA-1) and stores full hashes. -const FULL_COMMIT_HASH = /^[0-9a-f]{40}$/ +// `git init` honours the user's init.defaultObjectFormat, so full hashes are SHA-1 or SHA-256. +const FULL_COMMIT_HASH = /^(?:[0-9a-f]{40}|[0-9a-f]{64})$/ // The hash arrives from the webview, so it must name a checkpoint this task recorded before reaching Git. function assertRecordedCheckpoint(task: Task, commitHash: unknown): asserts commitHash is string { From 8a838907318c76f7a7a0a9c938b676815f58bd27 Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Sun, 1 Nov 2026 06:17:43 -0800 Subject: [PATCH 12/15] test: cover SHA-256 checkpoint diff and invalid-hash locale contract --- .../checkpoints/__tests__/checkpoint.test.ts | 16 ++++++++++++++++ .../__tests__/checkpoint-translations.spec.ts | 7 ++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/core/checkpoints/__tests__/checkpoint.test.ts b/src/core/checkpoints/__tests__/checkpoint.test.ts index b7e9931de6..1d56126f0a 100644 --- a/src/core/checkpoints/__tests__/checkpoint.test.ts +++ b/src/core/checkpoints/__tests__/checkpoint.test.ts @@ -431,6 +431,21 @@ describe("Checkpoint functionality", () => { expect(vscode.commands.executeCommand).not.toHaveBeenCalled() }) + it("should show diff for a recorded SHA-256 checkpoint hash", async () => { + const sha256Hash = "d".repeat(64) + mockTask.clineMessages.push({ ts: 5, say: "checkpoint_saved", text: sha256Hash }) + mockCheckpointService.getDiff.mockResolvedValue([ + { + paths: { absolute: "/test/file.ts", relative: "file.ts" }, + content: { before: "old content", after: "new content" }, + }, + ]) + + await checkpointDiff(mockTask, { ts: 5, commitHash: sha256Hash, mode: "to-current" }) + + expect(mockCheckpointService.getDiff).toHaveBeenCalledWith({ from: sha256Hash, to: undefined }) + }) + it.each([ ["a malformed hash", "HEAD~1"], ["a well-formed but unrecorded hash", "b".repeat(40)], @@ -439,6 +454,7 @@ describe("Checkpoint functionality", () => { await checkpointDiff(mockTask, { commitHash: commitHash as string, mode: "to-current" }) expect(vscode.window.showErrorMessage).toHaveBeenCalledWith("common:errors.checkpoint_invalid_hash") + expect(RepoPerTaskCheckpointService.create).not.toHaveBeenCalled() expect(mockCheckpointService.getDiff).not.toHaveBeenCalled() expect(mockTask.enableCheckpoints).toBe(true) }) diff --git a/src/i18n/__tests__/checkpoint-translations.spec.ts b/src/i18n/__tests__/checkpoint-translations.spec.ts index 281e73826a..4c21887a2b 100644 --- a/src/i18n/__tests__/checkpoint-translations.spec.ts +++ b/src/i18n/__tests__/checkpoint-translations.spec.ts @@ -25,7 +25,7 @@ const locales = [ ] interface CheckpointTranslations { - errors: { checkpoint_restore_failed: string; checkpoint_missing: string } + errors: { checkpoint_restore_failed: string; checkpoint_missing: string; checkpoint_invalid_hash: string } checkpoints: { restart_after_restore_failure: string; restart: string } } @@ -52,6 +52,11 @@ describe("checkpoint translations", () => { english.errors.checkpoint_restore_failed, ], ["errors.checkpoint_missing", translations.errors.checkpoint_missing, english.errors.checkpoint_missing], + [ + "errors.checkpoint_invalid_hash", + translations.errors.checkpoint_invalid_hash, + english.errors.checkpoint_invalid_hash, + ], [ "checkpoints.restart_after_restore_failure", translations.checkpoints.restart_after_restore_failure, From 77100241ec2509569c74afc6d6f1ffbafe290862 Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Sun, 1 Nov 2026 11:46:21 -0800 Subject: [PATCH 13/15] fix: remove listener bookkeeping for a rehydrated task discarded by the restart guard --- src/core/webview/ClineProvider.ts | 22 +++++++++---------- .../checkpointRestoreHandler.spec.ts | 6 +++++ 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 7a6a79322e..d08b2827ca 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -634,6 +634,14 @@ export class ClineProvider } } + private removeTaskEventListeners(task: Task): void { + const cleanupFunctions = this.taskEventListeners.get(task) + if (cleanupFunctions) { + cleanupFunctions.forEach((cleanup) => cleanup()) + this.taskEventListeners.delete(task) + } + } + private async cleanupFailedHistoryTask(task: Task, error: unknown): Promise { if (!(error instanceof PendingActionSettlementError)) { return @@ -646,11 +654,7 @@ export class ClineProvider this.taskRegistry.remove(task.taskId) task.emit(RooCodeEventName.TaskUnfocused) - const cleanupFunctions = this.taskEventListeners.get(task) - if (cleanupFunctions) { - cleanupFunctions.forEach((cleanup) => cleanup()) - this.taskEventListeners.delete(task) - } + this.removeTaskEventListeners(task) try { await task.dispose() @@ -1455,6 +1459,7 @@ export class ClineProvider // The current task may have changed or been removed during the awaits above; never touch it. if (oldTask?.taskId !== task.taskId) { + this.removeTaskEventListeners(task) await task.abortTask(true).catch(() => {}) return task } @@ -1469,12 +1474,7 @@ export class ClineProvider ) } - // Remove event listeners from the old task - const cleanupFunctions = this.taskEventListeners.get(oldTask) - if (cleanupFunctions) { - cleanupFunctions.forEach((cleanup) => cleanup()) - this.taskEventListeners.delete(oldTask) - } + this.removeTaskEventListeners(oldTask) // Replace in-place: preserves stack index and current pointer this.taskRegistry.replace(oldTask.taskId, task) diff --git a/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts b/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts index e736722263..2cdb88a82c 100644 --- a/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts +++ b/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts @@ -93,6 +93,7 @@ function createFixture() { setPendingEditOperation: vi.fn(), clearPendingEditOperation: vi.fn(), getPendingEditOperation: ClineProvider.prototype["getPendingEditOperation"], + removeTaskEventListeners: ClineProvider.prototype["removeTaskEventListeners"], getState: vi.fn(async () => ({ apiConfiguration: {} })), contextProxy: { globalStorageUri: { fsPath: "/test/storage" } }, taskScheduler: { schedule: vi.fn<(task: Task, run: () => Promise) => Promise>(async () => {}) }, @@ -278,6 +279,9 @@ describe("checkpointRestoreHandler", () => { const newerTask = { taskId: "newer-task", abortTask: vi.fn() } const constructedTask = { taskId: fixture.task.taskId, abortTask: vi.fn(async () => {}) } constructTask.mockReturnValueOnce(constructedTask) + const listenerCleanup = vi.fn() + const orphanTask = constructedTask as unknown as Task // Map key only; identity is what matters. + fixture.providerDouble.taskEventListeners.set(orphanTask, [listenerCleanup]) const registry = fixture.providerDouble.taskRegistry fixture.taskDouble.checkpointRestore.mockRejectedValueOnce(new Error("Missing commit")) showWarningMessage.mockResolvedValueOnce("common:checkpoints.restart") @@ -292,6 +296,8 @@ describe("checkpointRestoreHandler", () => { expect(newerTask.abortTask).not.toHaveBeenCalled() expect(registry.replace).not.toHaveBeenCalled() expect(fixture.providerDouble.taskScheduler.schedule).not.toHaveBeenCalled() + expect(listenerCleanup).toHaveBeenCalledOnce() + expect(fixture.providerDouble.taskEventListeners.has(orphanTask)).toBe(false) }) it.each([ From ba408c1b82b9737a88ed5c916bb8947a37aff433 Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Sun, 1 Nov 2026 12:58:00 -0800 Subject: [PATCH 14/15] fix: make task-listener cleanup a module helper so provider doubles keep working --- src/core/webview/ClineProvider.ts | 22 +++++++++---------- .../checkpointRestoreHandler.spec.ts | 1 - 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index d08b2827ca..d03ee4a9c1 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -209,6 +209,14 @@ type GetStateOptions = { includeTaskHistory?: boolean } +function removeTaskEventListeners(listeners: WeakMap void>>, task: Task): void { + const cleanupFunctions = listeners.get(task) + if (cleanupFunctions) { + cleanupFunctions.forEach((cleanup) => cleanup()) + listeners.delete(task) + } +} + export class ClineProvider extends EventEmitter implements vscode.WebviewViewProvider, TelemetryPropertiesProvider, TaskProviderLike @@ -634,14 +642,6 @@ export class ClineProvider } } - private removeTaskEventListeners(task: Task): void { - const cleanupFunctions = this.taskEventListeners.get(task) - if (cleanupFunctions) { - cleanupFunctions.forEach((cleanup) => cleanup()) - this.taskEventListeners.delete(task) - } - } - private async cleanupFailedHistoryTask(task: Task, error: unknown): Promise { if (!(error instanceof PendingActionSettlementError)) { return @@ -654,7 +654,7 @@ export class ClineProvider this.taskRegistry.remove(task.taskId) task.emit(RooCodeEventName.TaskUnfocused) - this.removeTaskEventListeners(task) + removeTaskEventListeners(this.taskEventListeners, task) try { await task.dispose() @@ -1459,7 +1459,7 @@ export class ClineProvider // The current task may have changed or been removed during the awaits above; never touch it. if (oldTask?.taskId !== task.taskId) { - this.removeTaskEventListeners(task) + removeTaskEventListeners(this.taskEventListeners, task) await task.abortTask(true).catch(() => {}) return task } @@ -1474,7 +1474,7 @@ export class ClineProvider ) } - this.removeTaskEventListeners(oldTask) + removeTaskEventListeners(this.taskEventListeners, oldTask) // Replace in-place: preserves stack index and current pointer this.taskRegistry.replace(oldTask.taskId, task) diff --git a/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts b/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts index 2cdb88a82c..64752e3317 100644 --- a/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts +++ b/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts @@ -93,7 +93,6 @@ function createFixture() { setPendingEditOperation: vi.fn(), clearPendingEditOperation: vi.fn(), getPendingEditOperation: ClineProvider.prototype["getPendingEditOperation"], - removeTaskEventListeners: ClineProvider.prototype["removeTaskEventListeners"], getState: vi.fn(async () => ({ apiConfiguration: {} })), contextProxy: { globalStorageUri: { fsPath: "/test/storage" } }, taskScheduler: { schedule: vi.fn<(task: Task, run: () => Promise) => Promise>(async () => {}) }, From 94711ae22baaa9ca7118b126d77ac48a309f1d39 Mon Sep 17 00:00:00 2001 From: Bertan Ari Date: Mon, 2 Nov 2026 08:41:04 -0800 Subject: [PATCH 15/15] fix: validate restore hash before cancelling task; compare task instance in rehydration guard --- src/core/checkpoints/index.ts | 17 +------ src/core/checkpoints/recordedCheckpoint.ts | 20 +++++++++ src/core/webview/ClineProvider.ts | 2 +- .../checkpointRestoreHandler.spec.ts | 8 +++- .../webviewMessageHandler.checkpoint.spec.ts | 45 ++++++++++++++++--- src/core/webview/webviewMessageHandler.ts | 23 ++++++---- 6 files changed, 82 insertions(+), 33 deletions(-) create mode 100644 src/core/checkpoints/recordedCheckpoint.ts diff --git a/src/core/checkpoints/index.ts b/src/core/checkpoints/index.ts index 77a67e5c91..0bf73a726e 100644 --- a/src/core/checkpoints/index.ts +++ b/src/core/checkpoints/index.ts @@ -16,6 +16,8 @@ import { DIFF_VIEW_URI_SCHEME } from "../../integrations/editor/DiffViewProvider import { CheckpointServiceOptions, RepoPerTaskCheckpointService } from "../../services/checkpoints" +import { assertRecordedCheckpoint } from "./recordedCheckpoint" + const WARNING_THRESHOLD_MS = 5000 function sendCheckpointInitWarn(task: Task, type?: "WAIT_TIMEOUT" | "INIT_TIMEOUT", timeout?: number) { @@ -227,21 +229,6 @@ export async function checkpointSave(task: Task, force = false, suppressMessage }) } -// `git init` honours the user's init.defaultObjectFormat, so full hashes are SHA-1 or SHA-256. -const FULL_COMMIT_HASH = /^(?:[0-9a-f]{40}|[0-9a-f]{64})$/ - -// The hash arrives from the webview, so it must name a checkpoint this task recorded before reaching Git. -function assertRecordedCheckpoint(task: Task, commitHash: unknown): asserts commitHash is string { - const isRecorded = - typeof commitHash === "string" && - FULL_COMMIT_HASH.test(commitHash) && - task.clineMessages.some(({ say, text }) => say === "checkpoint_saved" && text === commitHash) - - if (!isRecorded) { - throw new Error(t("common:errors.checkpoint_invalid_hash")) - } -} - export type CheckpointRestoreOptions = { ts: number commitHash: string diff --git a/src/core/checkpoints/recordedCheckpoint.ts b/src/core/checkpoints/recordedCheckpoint.ts new file mode 100644 index 0000000000..75386cb004 --- /dev/null +++ b/src/core/checkpoints/recordedCheckpoint.ts @@ -0,0 +1,20 @@ +import type { Task } from "../task/Task" +import { t } from "../../i18n" + +// `git init` honours the user's init.defaultObjectFormat, so full hashes are SHA-1 or SHA-256. +const FULL_COMMIT_HASH = /^(?:[0-9a-f]{40}|[0-9a-f]{64})$/ + +// The hash arrives from the webview, so it must name a checkpoint this task recorded before reaching Git. +export function assertRecordedCheckpoint( + task: Pick, + commitHash: unknown, +): asserts commitHash is string { + const isRecorded = + typeof commitHash === "string" && + FULL_COMMIT_HASH.test(commitHash) && + task.clineMessages.some(({ say, text }) => say === "checkpoint_saved" && text === commitHash) + + if (!isRecorded) { + throw new Error(t("common:errors.checkpoint_invalid_hash")) + } +} diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index d03ee4a9c1..6d16909f7e 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -1458,7 +1458,7 @@ export class ClineProvider const oldTask = this.taskRegistry.current // The current task may have changed or been removed during the awaits above; never touch it. - if (oldTask?.taskId !== task.taskId) { + if (oldTask !== currentTask) { removeTaskEventListeners(this.taskEventListeners, task) await task.abortTask(true).catch(() => {}) return task diff --git a/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts b/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts index 64752e3317..6195be5572 100644 --- a/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts +++ b/src/core/webview/__tests__/checkpointRestoreHandler.spec.ts @@ -273,9 +273,13 @@ describe("checkpointRestoreHandler", () => { it.each([ { name: "a different task became current", replacement: "newer" }, + { name: "a newer instance with the same id became current", replacement: "sameId" }, { name: "no task remains current", replacement: "none" }, ] as const)("restart skips replacement and scheduling when $name during setup", async ({ replacement }) => { - const newerTask = { taskId: "newer-task", abortTask: vi.fn() } + const newerTask = { + taskId: replacement === "sameId" ? fixture.task.taskId : "newer-task", + abortTask: vi.fn(), + } const constructedTask = { taskId: fixture.task.taskId, abortTask: vi.fn(async () => {}) } constructTask.mockReturnValueOnce(constructedTask) const listenerCleanup = vi.fn() @@ -285,7 +289,7 @@ describe("checkpointRestoreHandler", () => { fixture.taskDouble.checkpointRestore.mockRejectedValueOnce(new Error("Missing commit")) showWarningMessage.mockResolvedValueOnce("common:checkpoints.restart") fixture.providerDouble.getState.mockImplementationOnce(async () => { - registry.current = replacement === "newer" ? newerTask : undefined + registry.current = replacement === "none" ? undefined : newerTask return { apiConfiguration: {} } }) await expect(handleCheckpointRestoreOperation(fixture.config)).rejects.toThrow("Missing commit") diff --git a/src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts b/src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts index b15a7c9f48..5cb791026b 100644 --- a/src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts +++ b/src/core/webview/__tests__/webviewMessageHandler.checkpoint.spec.ts @@ -36,6 +36,8 @@ vi.mock("../../../i18n", () => ({ ), })) +const LATEST_CHECKPOINT = "b".repeat(40) + function createFixture() { const taskDouble = { taskId: "test-task-123", @@ -45,7 +47,7 @@ function createFixture() { { ts: 1, type: "say", say: "text", text: "Initial task" }, { ts: 2, type: "say", say: "checkpoint_saved", text: "initial-checkpoint" }, { ts: 3, type: "say", say: "user_feedback", text: "Latest prompt" }, - { ts: 4, type: "say", say: "checkpoint_saved", text: "latest-prompt-checkpoint" }, + { ts: 4, type: "say", say: "checkpoint_saved", text: LATEST_CHECKPOINT }, { ts: 5, type: "say", say: "completion_result", text: "Task complete" }, { ts: 6, type: "ask", ask: "completion_result", text: "", partial: false }, ] satisfies Task["clineMessages"], @@ -167,7 +169,7 @@ describe("webviewMessageHandler - checkpoint operations", () => { expect(taskDouble.abortTask).toHaveBeenCalledOnce() expect(taskDouble.checkpointRestore).toHaveBeenCalledWith({ ts: 3, - commitHash: "latest-prompt-checkpoint", + commitHash: LATEST_CHECKPOINT, mode: "restore", operation: "delete", }) @@ -227,7 +229,7 @@ describe("webviewMessageHandler - checkpoint operations", () => { expect(taskDouble.checkpointRestore).toHaveBeenCalledExactlyOnceWith({ ts: 3, - commitHash: "latest-prompt-checkpoint", + commitHash: LATEST_CHECKPOINT, mode: "restore", operation: "edit", }) @@ -290,14 +292,14 @@ describe("webviewMessageHandler - checkpoint operations", () => { fixture.taskDouble.checkpointRestore.mockRejectedValueOnce(failure) const message: WebviewMessage = type === "checkpointRestore" - ? { type, payload: { ts: 4, commitHash: "latest-prompt-checkpoint", mode: "restore" } } + ? { type, payload: { ts: 4, commitHash: LATEST_CHECKPOINT, mode: "restore" } } : { type } await webviewMessageHandler(fixture.provider, message) expect(fixture.providerDouble.cancelTask).toHaveBeenCalledOnce() expect(fixture.taskDouble.checkpointRestore).toHaveBeenCalledWith({ ts: 4, - commitHash: "latest-prompt-checkpoint", + commitHash: LATEST_CHECKPOINT, mode: "restore", }) expect(console.error).toHaveBeenCalledWith(`[${type}] checkpointRestore failed:`, failure) @@ -309,12 +311,41 @@ describe("webviewMessageHandler - checkpoint operations", () => { expectUnchanged(messages, history) }) + it.each([ + { name: "malformed", commitHash: "HEAD~1" }, + { name: "unrecorded", commitHash: "c".repeat(40) }, + ])("checkpointRestore rejects a $name hash without cancelling the task", async ({ commitHash }) => { + const messages = structuredClone(fixture.task.clineMessages) + const history = structuredClone(fixture.task.apiConversationHistory) + await webviewMessageHandler(fixture.provider, { + type: "checkpointRestore", + payload: { ts: 4, commitHash, mode: "restore" }, + }) + + expect(fixture.providerDouble.cancelTask).not.toHaveBeenCalled() + expect(fixture.taskDouble.checkpointRestore).not.toHaveBeenCalled() + expect(vscode.window.showErrorMessage).toHaveBeenCalledExactlyOnceWith( + "common:errors.checkpoint_restore_failed: common:errors.checkpoint_invalid_hash", + ) + expectUnchanged(messages, history) + }) + + it("checkpointRestore cancels before restoring a recorded hash", async () => { + await webviewMessageHandler(fixture.provider, { + type: "checkpointRestore", + payload: { ts: 4, commitHash: LATEST_CHECKPOINT, mode: "restore" }, + }) + expect(fixture.providerDouble.cancelTask.mock.invocationCallOrder[0]).toBeLessThan( + fixture.taskDouble.checkpointRestore.mock.invocationCallOrder[0], + ) + }) + describe("completion checkpoint actions", () => { it("diffs the checkpoint created after the latest prompt", async () => { await webviewMessageHandler(fixture.provider, { type: "completionCheckpointDiff" }) expect(fixture.taskDouble.checkpointDiff).toHaveBeenCalledWith({ ts: 4, - commitHash: "latest-prompt-checkpoint", + commitHash: LATEST_CHECKPOINT, mode: "to-current", }) }) @@ -323,7 +354,7 @@ describe("webviewMessageHandler - checkpoint operations", () => { await webviewMessageHandler(fixture.provider, { type: "completionCheckpointRestore" }) expect(fixture.taskDouble.checkpointRestore).toHaveBeenCalledWith({ ts: 4, - commitHash: "latest-prompt-checkpoint", + commitHash: LATEST_CHECKPOINT, mode: "restore", }) expect(fixture.providerDouble.cancelTask.mock.invocationCallOrder[0]).toBeLessThan( diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 41a268314e..227381ea22 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -42,6 +42,7 @@ import { importRooTaskHistory } from "../task-persistence/importRooTaskHistory" import { ClineProvider } from "./ClineProvider" import { findOriginalContent } from "./stripOriginalContent" import { handleCheckpointRestoreOperation } from "./checkpointRestoreHandler" +import { assertRecordedCheckpoint } from "../checkpoints/recordedCheckpoint" import { generateErrorDiagnostics } from "./diagnosticsHandler" import { handleRequestSkills, @@ -1784,16 +1785,22 @@ export const webviewMessageHandler = async ( const result = checkoutRestorePayloadSchema.safeParse(message.payload) if (result.success) { - await provider.cancelTask() - try { - await pWaitFor(() => provider.getCurrentTask()?.isInitialized === true, { timeout: 3_000 }) - } catch { - vscode.window.showErrorMessage(t("common:errors.checkpoint_timeout")) - return - } + const taskBeforeCancel = provider.getCurrentTask() + // Reject a bad hash before cancelling, so the running task survives. + if (taskBeforeCancel) { + assertRecordedCheckpoint(taskBeforeCancel, result.data.commitHash) + } + + await provider.cancelTask() + + try { + await pWaitFor(() => provider.getCurrentTask()?.isInitialized === true, { timeout: 3_000 }) + } catch { + vscode.window.showErrorMessage(t("common:errors.checkpoint_timeout")) + return + } - try { await provider.getCurrentTask()?.checkpointRestore(result.data) } catch (error) { console.error("[checkpointRestore] checkpointRestore failed:", error)