From 68ff869bdd08dcd883411fffd230628d2af4be95 Mon Sep 17 00:00:00 2001 From: Amp Date: Wed, 16 Sep 2026 12:13:04 +0000 Subject: [PATCH 1/8] fix(history): prompt on workspace mismatch Amp-Thread-ID: https://ampcode.com/threads/T-01a0aa02-b6de-7763-97b8-1650ea4b46da --- src/core/webview/ClineProvider.ts | 59 +++++++- .../ClineProvider.history-workspace.spec.ts | 134 ++++++++++++++++++ src/extension/api.ts | 6 +- 3 files changed, 196 insertions(+), 3 deletions(-) create mode 100644 src/core/webview/__tests__/ClineProvider.history-workspace.spec.ts diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 28a42913f3..57d6a20808 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -101,7 +101,7 @@ import { SkillsManager } from "../../services/skills/SkillsManager" import { fileExistsAtPath } from "../../utils/fs" import { setTtsEnabled, setTtsSpeed } from "../../utils/tts" import { getWorkspaceGitInfo } from "../../utils/git" -import { getWorkspacePath } from "../../utils/path" +import { arePathsEqual, getWorkspacePath } from "../../utils/path" import { OrganizationAllowListViolationError } from "../../utils/errors" import { setPanel } from "../../activate/registerCommands" @@ -2430,12 +2430,67 @@ export class ClineProvider if (id !== this.getCurrentTask()?.taskId) { // Non-current task. const { historyItem } = await this.getTaskWithId(id) - await this.createTaskWithHistoryItem(historyItem) // Clears existing task. + const preparedHistoryItem = await this.prepareHistoryItemForResume(historyItem) + if (!preparedHistoryItem) { + return + } + await this.createTaskWithHistoryItem(preparedHistoryItem) // Clears existing task. } await this.postMessageToWebview({ type: "action", action: "chatButtonClicked" }) } + public async prepareHistoryItemForResume(historyItem: T): Promise { + const currentWorkspace = this.cwd + const originalWorkspace = historyItem.workspace + + if (!currentWorkspace || !originalWorkspace || arePathsEqual(currentWorkspace, originalWorkspace)) { + return historyItem + } + + const useCurrentWorkspace = { title: "Use Current Workspace" } + const openOriginalWorkspace = { title: "Open Original Workspace" } + const selection = await vscode.window.showWarningMessage( + `This conversation was created in "${originalWorkspace}", but the current workspace is "${currentWorkspace}". ` + + "Choose where to continue. Using the current workspace resets checkpoints created in the original workspace.", + { modal: true }, + useCurrentWorkspace, + openOriginalWorkspace, + ) + + if (selection?.title === openOriginalWorkspace.title) { + await vscode.commands.executeCommand("vscode.openFolder", vscode.Uri.file(originalWorkspace), { + forceNewWindow: true, + }) + return undefined + } + + if (selection?.title !== useCurrentWorkspace.title) { + return undefined + } + + await this.resetTaskCheckpointsForWorkspaceChange(historyItem.id) + const updatedHistoryItem = { ...historyItem, workspace: currentWorkspace } + await this.updateTaskHistory(updatedHistoryItem) + return updatedHistoryItem + } + + private async resetTaskCheckpointsForWorkspaceChange(taskId: string): Promise { + const globalStoragePath = this.contextProxy.globalStorageUri.fsPath + const messages = await readTaskMessages({ taskId, globalStoragePath }) + const messagesWithoutCheckpoints = messages.filter( + (message) => !(message.type === "say" && message.say === "checkpoint_saved"), + ) + const { getTaskDirectoryPath } = await import("../../utils/storage") + const taskDir = await getTaskDirectoryPath(globalStoragePath, taskId) + + await fs.rm(path.join(taskDir, "checkpoints"), { recursive: true, force: true }) + + if (messagesWithoutCheckpoints.length !== messages.length) { + await saveTaskMessages({ messages: messagesWithoutCheckpoints, taskId, globalStoragePath }) + } + } + async exportTaskWithId(id: string) { const { historyItem, apiConversationHistory } = await this.getTaskWithId(id) const fileName = getTaskFileName(historyItem.ts) diff --git a/src/core/webview/__tests__/ClineProvider.history-workspace.spec.ts b/src/core/webview/__tests__/ClineProvider.history-workspace.spec.ts new file mode 100644 index 0000000000..d8f04a6cdf --- /dev/null +++ b/src/core/webview/__tests__/ClineProvider.history-workspace.spec.ts @@ -0,0 +1,134 @@ +import fs from "fs/promises" +import os from "os" +import path from "path" +import * as vscode from "vscode" + +import type { HistoryItem } from "@roo-code/types" + +import { ClineProvider } from "../ClineProvider" + +const historyItem = (workspace: string): HistoryItem => ({ + id: "task-1602", + number: 1, + ts: 1, + task: "Continue in another worktree", + tokensIn: 0, + tokensOut: 0, + cacheWrites: 0, + cacheReads: 0, + totalCost: 0, + workspace, +}) + +const createProvider = (workspace: string) => { + const provider = Object.create(ClineProvider.prototype) as ClineProvider + Object.defineProperty(provider, "currentWorkspacePath", { value: workspace, writable: true }) + return provider +} + +describe("ClineProvider historical workspace selection", () => { + afterEach(() => { + vi.restoreAllMocks() + }) + + it("keeps a history item unchanged when it already belongs to the current workspace", async () => { + const provider = createProvider("/current/workspace") + const prompt = vi.spyOn(vscode.window, "showWarningMessage") + const item = historyItem("/current/workspace") + + await expect(provider.prepareHistoryItemForResume(item)).resolves.toBe(item) + expect(prompt).not.toHaveBeenCalled() + }) + + it("cancels restoration without mutating history when the mismatch prompt is dismissed", async () => { + const provider = createProvider("/current/workspace") + vi.spyOn(vscode.window, "showWarningMessage").mockResolvedValue(undefined) + provider["resetTaskCheckpointsForWorkspaceChange"] = vi.fn() + provider.updateTaskHistory = vi.fn() + + await expect(provider.prepareHistoryItemForResume(historyItem("/old/worktree"))).resolves.toBeUndefined() + expect(provider["resetTaskCheckpointsForWorkspaceChange"]).not.toHaveBeenCalled() + expect(provider.updateTaskHistory).not.toHaveBeenCalled() + }) + + it("opens the original workspace in a new window without restoring the task", async () => { + const provider = createProvider("/current/workspace") + vi.spyOn(vscode.window, "showWarningMessage").mockResolvedValue({ title: "Open Original Workspace" }) + const executeCommand = vi.spyOn(vscode.commands, "executeCommand").mockResolvedValue(undefined) + + await expect(provider.prepareHistoryItemForResume(historyItem("/old/worktree"))).resolves.toBeUndefined() + expect(executeCommand).toHaveBeenCalledWith( + "vscode.openFolder", + expect.objectContaining({ fsPath: "/old/worktree" }), + { forceNewWindow: true }, + ) + }) + + it("moves the task to the current workspace and resets workspace-specific checkpoints", async () => { + const provider = createProvider("/current/workspace") + vi.spyOn(vscode.window, "showWarningMessage").mockResolvedValue({ title: "Use Current Workspace" }) + provider["resetTaskCheckpointsForWorkspaceChange"] = vi.fn().mockResolvedValue(undefined) + provider.updateTaskHistory = vi.fn().mockResolvedValue([]) + const item = historyItem("/old/worktree") + + await expect(provider.prepareHistoryItemForResume(item)).resolves.toEqual({ + ...item, + workspace: "/current/workspace", + }) + expect(provider["resetTaskCheckpointsForWorkspaceChange"]).toHaveBeenCalledWith(item.id) + expect(provider.updateTaskHistory).toHaveBeenCalledWith({ ...item, workspace: "/current/workspace" }) + }) + + it("restores history with the workspace selected by the mismatch prompt", async () => { + const provider = createProvider("/current/workspace") + const original = historyItem("/old/worktree") + const prepared = { ...original, workspace: "/current/workspace" } + provider.getCurrentTask = vi.fn().mockReturnValue(undefined) + provider.getTaskWithId = vi.fn().mockResolvedValue({ historyItem: original }) + provider.prepareHistoryItemForResume = vi.fn().mockResolvedValue(prepared) + provider.createTaskWithHistoryItem = vi.fn().mockResolvedValue({}) + provider.postMessageToWebview = vi.fn().mockResolvedValue(true) + + await provider.showTaskWithId(original.id) + + expect(provider.createTaskWithHistoryItem).toHaveBeenCalledWith(prepared) + expect(provider.postMessageToWebview).toHaveBeenCalledWith({ + type: "action", + action: "chatButtonClicked", + }) + }) + + it("removes the old checkpoint repository and checkpoint-only chat rows", async () => { + const storagePath = await fs.mkdtemp(path.join(os.tmpdir(), "zoo-history-workspace-")) + const taskDir = path.join(storagePath, "tasks", "task-1602") + const checkpointsDir = path.join(taskDir, "checkpoints") + await fs.mkdir(checkpointsDir, { recursive: true }) + await fs.writeFile(path.join(checkpointsDir, "HEAD"), "old checkpoint") + await fs.writeFile( + path.join(taskDir, "ui_messages.json"), + JSON.stringify([ + { type: "say", say: "task", ts: 1, text: "Continue" }, + { type: "say", say: "checkpoint_saved", ts: 2, text: "old-hash" }, + { type: "say", say: "text", ts: 3, text: "Still useful" }, + ]), + ) + + const provider = createProvider("/current/workspace") + Object.defineProperty(provider, "contextProxy", { + value: { globalStorageUri: { fsPath: storagePath } }, + }) + + try { + await provider["resetTaskCheckpointsForWorkspaceChange"]("task-1602") + + await expect(fs.stat(checkpointsDir)).rejects.toMatchObject({ code: "ENOENT" }) + const messages = JSON.parse(await fs.readFile(path.join(taskDir, "ui_messages.json"), "utf8")) + expect(messages).toMatchObject([ + { type: "say", say: "task", text: "Continue" }, + { type: "say", say: "text", text: "Still useful" }, + ]) + } finally { + await fs.rm(storagePath, { recursive: true, force: true }) + } + }) +}) diff --git a/src/extension/api.ts b/src/extension/api.ts index 3a33510346..2234d914e8 100644 --- a/src/extension/api.ts +++ b/src/extension/api.ts @@ -266,7 +266,11 @@ export class API extends EventEmitter implements RooCodeAPI { await this.waitForWebviewLaunch(5_000) const { historyItem } = await this.sidebarProvider.getTaskWithId(taskId) - await this.sidebarProvider.createTaskWithHistoryItem(historyItem) + const preparedHistoryItem = await this.sidebarProvider.prepareHistoryItemForResume(historyItem) + if (!preparedHistoryItem) { + return + } + await this.sidebarProvider.createTaskWithHistoryItem(preparedHistoryItem) if (this.sidebarProvider.viewLaunched) { await this.sidebarProvider.postMessageToWebview({ type: "action", action: "chatButtonClicked" }) From 9d6aefa24afee074cbd2f85b25eab797c08f7758 Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Wed, 16 Sep 2026 17:04:36 +0200 Subject: [PATCH 2/8] fix(history): make workspace reset recoverable --- src/core/webview/ClineProvider.ts | 44 ++++++++++-- .../ClineProvider.history-workspace.spec.ts | 71 +++++++++++++++++-- .../__tests__/api-resume-task.spec.ts | 33 +++++++++ 3 files changed, 137 insertions(+), 11 deletions(-) create mode 100644 src/extension/__tests__/api-resume-task.spec.ts diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 57d6a20808..2c9d0b9bf6 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -103,6 +103,7 @@ import { setTtsEnabled, setTtsSpeed } from "../../utils/tts" import { getWorkspaceGitInfo } from "../../utils/git" import { arePathsEqual, getWorkspacePath } from "../../utils/path" import { OrganizationAllowListViolationError } from "../../utils/errors" +import { getTaskDirectoryPath } from "../../utils/storage" import { setPanel } from "../../activate/registerCommands" @@ -2469,25 +2470,54 @@ export class ClineProvider return undefined } - await this.resetTaskCheckpointsForWorkspaceChange(historyItem.id) const updatedHistoryItem = { ...historyItem, workspace: currentWorkspace } - await this.updateTaskHistory(updatedHistoryItem) + await this.resetTaskCheckpointsForWorkspaceChange(historyItem, updatedHistoryItem) return updatedHistoryItem } - private async resetTaskCheckpointsForWorkspaceChange(taskId: string): Promise { + private async resetTaskCheckpointsForWorkspaceChange( + originalHistoryItem: HistoryItem, + updatedHistoryItem: HistoryItem, + ): Promise { + const taskId = originalHistoryItem.id const globalStoragePath = this.contextProxy.globalStorageUri.fsPath const messages = await readTaskMessages({ taskId, globalStoragePath }) const messagesWithoutCheckpoints = messages.filter( (message) => !(message.type === "say" && message.say === "checkpoint_saved"), ) - const { getTaskDirectoryPath } = await import("../../utils/storage") const taskDir = await getTaskDirectoryPath(globalStoragePath, taskId) + const checkpointsDir = path.join(taskDir, "checkpoints") + const checkpointBackupDir = path.join(taskDir, `checkpoints.workspace-change-${crypto.randomUUID()}`) + let checkpointDirectoryStaged = false - await fs.rm(path.join(taskDir, "checkpoints"), { recursive: true, force: true }) + try { + await fs.rename(checkpointsDir, checkpointBackupDir) + checkpointDirectoryStaged = true + } catch (error) { + if (!(error instanceof Error && "code" in error && error.code === "ENOENT")) { + throw error + } + } - if (messagesWithoutCheckpoints.length !== messages.length) { - await saveTaskMessages({ messages: messagesWithoutCheckpoints, taskId, globalStoragePath }) + try { + if (messagesWithoutCheckpoints.length !== messages.length) { + await saveTaskMessages({ messages: messagesWithoutCheckpoints, taskId, globalStoragePath }) + } + await this.updateTaskHistory(updatedHistoryItem) + if (checkpointDirectoryStaged) { + await fs.rm(checkpointBackupDir, { recursive: true, force: true }) + } + } catch (error) { + if (messagesWithoutCheckpoints.length !== messages.length) { + await saveTaskMessages({ messages, taskId, globalStoragePath }) + } + if (checkpointDirectoryStaged) { + await fs.rename(checkpointBackupDir, checkpointsDir) + } + if (this.taskHistoryStore.get(taskId)?.workspace === updatedHistoryItem.workspace) { + await this.updateTaskHistory(originalHistoryItem) + } + throw error } } diff --git a/src/core/webview/__tests__/ClineProvider.history-workspace.spec.ts b/src/core/webview/__tests__/ClineProvider.history-workspace.spec.ts index d8f04a6cdf..f2ee88896e 100644 --- a/src/core/webview/__tests__/ClineProvider.history-workspace.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.history-workspace.spec.ts @@ -53,10 +53,18 @@ describe("ClineProvider historical workspace selection", () => { it("opens the original workspace in a new window without restoring the task", async () => { const provider = createProvider("/current/workspace") - vi.spyOn(vscode.window, "showWarningMessage").mockResolvedValue({ title: "Open Original Workspace" }) + const prompt = vi + .spyOn(vscode.window, "showWarningMessage") + .mockResolvedValue({ title: "Open Original Workspace" }) const executeCommand = vi.spyOn(vscode.commands, "executeCommand").mockResolvedValue(undefined) await expect(provider.prepareHistoryItemForResume(historyItem("/old/worktree"))).resolves.toBeUndefined() + expect(prompt).toHaveBeenCalledWith( + expect.any(String), + { modal: true }, + { title: "Use Current Workspace" }, + { title: "Open Original Workspace" }, + ) expect(executeCommand).toHaveBeenCalledWith( "vscode.openFolder", expect.objectContaining({ fsPath: "/old/worktree" }), @@ -75,8 +83,10 @@ describe("ClineProvider historical workspace selection", () => { ...item, workspace: "/current/workspace", }) - expect(provider["resetTaskCheckpointsForWorkspaceChange"]).toHaveBeenCalledWith(item.id) - expect(provider.updateTaskHistory).toHaveBeenCalledWith({ ...item, workspace: "/current/workspace" }) + expect(provider["resetTaskCheckpointsForWorkspaceChange"]).toHaveBeenCalledWith(item, { + ...item, + workspace: "/current/workspace", + }) }) it("restores history with the workspace selected by the mismatch prompt", async () => { @@ -98,6 +108,21 @@ describe("ClineProvider historical workspace selection", () => { }) }) + it("does not restore or reveal a task when workspace selection is cancelled", async () => { + const provider = createProvider("/current/workspace") + const original = historyItem("/old/worktree") + provider.getCurrentTask = vi.fn().mockReturnValue(undefined) + provider.getTaskWithId = vi.fn().mockResolvedValue({ historyItem: original }) + provider.prepareHistoryItemForResume = vi.fn().mockResolvedValue(undefined) + provider.createTaskWithHistoryItem = vi.fn() + provider.postMessageToWebview = vi.fn() + + await provider.showTaskWithId(original.id) + + expect(provider.createTaskWithHistoryItem).not.toHaveBeenCalled() + expect(provider.postMessageToWebview).not.toHaveBeenCalled() + }) + it("removes the old checkpoint repository and checkpoint-only chat rows", async () => { const storagePath = await fs.mkdtemp(path.join(os.tmpdir(), "zoo-history-workspace-")) const taskDir = path.join(storagePath, "tasks", "task-1602") @@ -117,9 +142,13 @@ describe("ClineProvider historical workspace selection", () => { Object.defineProperty(provider, "contextProxy", { value: { globalStorageUri: { fsPath: storagePath } }, }) + const original = historyItem("/old/worktree") + const updated = { ...original, workspace: "/current/workspace" } + provider.updateTaskHistory = vi.fn().mockResolvedValue([]) + Object.defineProperty(provider, "taskHistoryStore", { value: { get: vi.fn().mockReturnValue(original) } }) try { - await provider["resetTaskCheckpointsForWorkspaceChange"]("task-1602") + await provider["resetTaskCheckpointsForWorkspaceChange"](original, updated) await expect(fs.stat(checkpointsDir)).rejects.toMatchObject({ code: "ENOENT" }) const messages = JSON.parse(await fs.readFile(path.join(taskDir, "ui_messages.json"), "utf8")) @@ -127,6 +156,40 @@ describe("ClineProvider historical workspace selection", () => { { type: "say", say: "task", text: "Continue" }, { type: "say", say: "text", text: "Still useful" }, ]) + expect(provider.updateTaskHistory).toHaveBeenCalledWith(updated) + } finally { + await fs.rm(storagePath, { recursive: true, force: true }) + } + }) + + it("restores checkpoint files and messages when workspace persistence fails", async () => { + const storagePath = await fs.mkdtemp(path.join(os.tmpdir(), "zoo-history-workspace-rollback-")) + const taskDir = path.join(storagePath, "tasks", "task-1602") + const checkpointsDir = path.join(taskDir, "checkpoints") + const messagesPath = path.join(taskDir, "ui_messages.json") + const originalMessages = [ + { type: "say", say: "task", ts: 1, text: "Continue" }, + { type: "say", say: "checkpoint_saved", ts: 2, text: "old-hash" }, + ] + await fs.mkdir(checkpointsDir, { recursive: true }) + await fs.writeFile(path.join(checkpointsDir, "HEAD"), "old checkpoint") + await fs.writeFile(messagesPath, JSON.stringify(originalMessages)) + + const provider = createProvider("/current/workspace") + const original = historyItem("/old/worktree") + const updated = { ...original, workspace: "/current/workspace" } + Object.defineProperty(provider, "contextProxy", { + value: { globalStorageUri: { fsPath: storagePath } }, + }) + Object.defineProperty(provider, "taskHistoryStore", { value: { get: vi.fn().mockReturnValue(original) } }) + provider.updateTaskHistory = vi.fn().mockRejectedValue(new Error("history write failed")) + + try { + await expect(provider["resetTaskCheckpointsForWorkspaceChange"](original, updated)).rejects.toThrow( + "history write failed", + ) + await expect(fs.readFile(path.join(checkpointsDir, "HEAD"), "utf8")).resolves.toBe("old checkpoint") + expect(JSON.parse(await fs.readFile(messagesPath, "utf8"))).toMatchObject(originalMessages) } finally { await fs.rm(storagePath, { recursive: true, force: true }) } diff --git a/src/extension/__tests__/api-resume-task.spec.ts b/src/extension/__tests__/api-resume-task.spec.ts new file mode 100644 index 0000000000..8c4b5043ef --- /dev/null +++ b/src/extension/__tests__/api-resume-task.spec.ts @@ -0,0 +1,33 @@ +import * as vscode from "vscode" + +import type { HistoryItem } from "@roo-code/types" + +import { API } from "../api" +import type { ClineProvider } from "../../core/webview/ClineProvider" + +const historyItem = { + id: "task-1602", + task: "Resume task", + tokensIn: 0, + tokensOut: 0, + totalCost: 0, +} as HistoryItem + +describe("API.resumeTask", () => { + it("does not restore a task when workspace selection is cancelled", async () => { + const provider = { + viewLaunched: true, + context: {}, + getTaskWithId: vi.fn().mockResolvedValue({ historyItem }), + prepareHistoryItemForResume: vi.fn().mockResolvedValue(undefined), + createTaskWithHistoryItem: vi.fn(), + on: vi.fn(), + } as unknown as ClineProvider + const outputChannel = { appendLine: vi.fn() } as unknown as vscode.OutputChannel + const api = new API(outputChannel, provider) + + await api.resumeTask(historyItem.id) + + expect(provider.createTaskWithHistoryItem).not.toHaveBeenCalled() + }) +}) From f0cc2638bea390edde981f3acd663ec93cf44b05 Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Fri, 18 Sep 2026 15:33:58 +0200 Subject: [PATCH 3/8] fix(webview): isolate checkpoint cleanup failures --- src/core/webview/ClineProvider.ts | 13 ++++-- .../ClineProvider.history-workspace.spec.ts | 42 +++++++++++++++++++ 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 2c9d0b9bf6..644042e729 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -2504,9 +2504,6 @@ export class ClineProvider await saveTaskMessages({ messages: messagesWithoutCheckpoints, taskId, globalStoragePath }) } await this.updateTaskHistory(updatedHistoryItem) - if (checkpointDirectoryStaged) { - await fs.rm(checkpointBackupDir, { recursive: true, force: true }) - } } catch (error) { if (messagesWithoutCheckpoints.length !== messages.length) { await saveTaskMessages({ messages, taskId, globalStoragePath }) @@ -2519,6 +2516,16 @@ export class ClineProvider } throw error } + + if (checkpointDirectoryStaged) { + try { + await fs.rm(checkpointBackupDir, { recursive: true, force: true }) + } catch (error) { + this.log( + `[resetTaskCheckpointsForWorkspaceChange] Failed to remove checkpoint backup for ${taskId}: ${error instanceof Error ? error.message : String(error)}`, + ) + } + } } async exportTaskWithId(id: string) { diff --git a/src/core/webview/__tests__/ClineProvider.history-workspace.spec.ts b/src/core/webview/__tests__/ClineProvider.history-workspace.spec.ts index f2ee88896e..b84e52de73 100644 --- a/src/core/webview/__tests__/ClineProvider.history-workspace.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.history-workspace.spec.ts @@ -194,4 +194,46 @@ describe("ClineProvider historical workspace selection", () => { await fs.rm(storagePath, { recursive: true, force: true }) } }) + + it("keeps committed history when checkpoint backup cleanup fails", async () => { + const storagePath = await fs.mkdtemp(path.join(os.tmpdir(), "zoo-history-workspace-cleanup-")) + const taskDir = path.join(storagePath, "tasks", "task-1602") + const checkpointsDir = path.join(taskDir, "checkpoints") + const messagesPath = path.join(taskDir, "ui_messages.json") + await fs.mkdir(checkpointsDir, { recursive: true }) + await fs.writeFile(path.join(checkpointsDir, "HEAD"), "old checkpoint") + await fs.writeFile( + messagesPath, + JSON.stringify([ + { type: "say", say: "task", ts: 1, text: "Continue" }, + { type: "say", say: "checkpoint_saved", ts: 2, text: "old-hash" }, + ]), + ) + + const provider = createProvider("/current/workspace") + const original = historyItem("/old/worktree") + const updated = { ...original, workspace: "/current/workspace" } + Object.defineProperty(provider, "contextProxy", { + value: { globalStorageUri: { fsPath: storagePath } }, + }) + Object.defineProperty(provider, "taskHistoryStore", { value: { get: vi.fn().mockReturnValue(updated) } }) + provider.updateTaskHistory = vi.fn().mockResolvedValue([]) + provider["log"] = vi.fn() + vi.spyOn(fs, "rm").mockRejectedValueOnce(new Error("backup cleanup failed")) + + try { + await expect(provider["resetTaskCheckpointsForWorkspaceChange"](original, updated)).resolves.toBeUndefined() + + expect(provider.updateTaskHistory).toHaveBeenCalledOnce() + expect(provider.updateTaskHistory).toHaveBeenCalledWith(updated) + expect(provider["log"]).toHaveBeenCalledWith(expect.stringContaining("backup cleanup failed")) + await expect(fs.stat(checkpointsDir)).rejects.toMatchObject({ code: "ENOENT" }) + expect(JSON.parse(await fs.readFile(messagesPath, "utf8"))).toMatchObject([ + { type: "say", say: "task", text: "Continue" }, + ]) + } finally { + vi.mocked(fs.rm).mockRestore() + await fs.rm(storagePath, { recursive: true, force: true }) + } + }) }) From 6b91307508f53a05ab557c9b02150d83b3eb62ce Mon Sep 17 00:00:00 2001 From: Amp Date: Mon, 21 Sep 2026 18:30:09 +0000 Subject: [PATCH 4/8] test(visual): pin chat fixture token usage Amp-Thread-ID: https://ampcode.com/threads/T-01a0c52c-64bd-762d-8a8a-45bd3b2daba8 --- apps/vscode-e2e/src/visual/electron.visual.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/apps/vscode-e2e/src/visual/electron.visual.ts b/apps/vscode-e2e/src/visual/electron.visual.ts index 155f496980..affeb9b46d 100644 --- a/apps/vscode-e2e/src/visual/electron.visual.ts +++ b/apps/vscode-e2e/src/visual/electron.visual.ts @@ -122,6 +122,8 @@ async function startScene( mock.addFixture({ match: { userMessage: CHAT_PROMPT }, response: { + // Keep the context meter independent of system prompts and temporary workspace paths. + usage: { prompt_tokens: 4096, completion_tokens: 128, total_tokens: 4224 }, toolCalls: [ { name: "attempt_completion", @@ -230,6 +232,9 @@ for (const scenario of scenarios) { await contentFrame .getByText(scenario.landmark, { exact: false }) .waitFor({ state: "visible", timeout: 60_000 }) + if (scenario.scene === "chat") { + await expect(contentFrame.getByTestId("context-tokens-count")).toHaveText("4.2k") + } await contentFrame.evaluate(() => { if (document.activeElement instanceof HTMLElement) document.activeElement.blur() }) From 7448eff02a3324cab6aef5c8c1ffe17260e8f4f5 Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Mon, 5 Oct 2026 17:47:50 +0200 Subject: [PATCH 5/8] fix(history): isolate workspace rollback failures Attempt every restore step even when another rollback fails, log each failure, and preserve the original error. Add regression coverage for individual and combined rollback failures. Amp-Thread-ID: https://ampcode.com/threads/T-01a10c95-779b-756d-9944-c14dcc2f955e Co-authored-by: Amp --- src/core/webview/ClineProvider.ts | 26 +++++- .../ClineProvider.history-workspace.spec.ts | 83 +++++++++++++++++++ 2 files changed, 105 insertions(+), 4 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 644042e729..66a3021130 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -2506,13 +2506,31 @@ export class ClineProvider await this.updateTaskHistory(updatedHistoryItem) } catch (error) { if (messagesWithoutCheckpoints.length !== messages.length) { - await saveTaskMessages({ messages, taskId, globalStoragePath }) + try { + await saveTaskMessages({ messages, taskId, globalStoragePath }) + } catch (rollbackError) { + this.log( + `[resetTaskCheckpointsForWorkspaceChange] Failed to restore messages for ${taskId}: ${rollbackError instanceof Error ? rollbackError.message : String(rollbackError)}`, + ) + } } if (checkpointDirectoryStaged) { - await fs.rename(checkpointBackupDir, checkpointsDir) + try { + await fs.rename(checkpointBackupDir, checkpointsDir) + } catch (rollbackError) { + this.log( + `[resetTaskCheckpointsForWorkspaceChange] Failed to restore checkpoints for ${taskId} from ${checkpointBackupDir}: ${rollbackError instanceof Error ? rollbackError.message : String(rollbackError)}`, + ) + } } - if (this.taskHistoryStore.get(taskId)?.workspace === updatedHistoryItem.workspace) { - await this.updateTaskHistory(originalHistoryItem) + try { + if (this.taskHistoryStore.get(taskId)?.workspace === updatedHistoryItem.workspace) { + await this.updateTaskHistory(originalHistoryItem) + } + } catch (rollbackError) { + this.log( + `[resetTaskCheckpointsForWorkspaceChange] Failed to restore history for ${taskId}: ${rollbackError instanceof Error ? rollbackError.message : String(rollbackError)}`, + ) } throw error } diff --git a/src/core/webview/__tests__/ClineProvider.history-workspace.spec.ts b/src/core/webview/__tests__/ClineProvider.history-workspace.spec.ts index b84e52de73..223ffe3482 100644 --- a/src/core/webview/__tests__/ClineProvider.history-workspace.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.history-workspace.spec.ts @@ -6,6 +6,7 @@ import * as vscode from "vscode" import type { HistoryItem } from "@roo-code/types" import { ClineProvider } from "../ClineProvider" +import * as taskMessages from "../../task-persistence/taskMessages" const historyItem = (workspace: string): HistoryItem => ({ id: "task-1602", @@ -195,6 +196,88 @@ describe("ClineProvider historical workspace selection", () => { } }) + it.each([ + { failures: ["messages"] }, + { failures: ["checkpoints"] }, + { failures: ["history"] }, + { failures: ["messages", "checkpoints", "history"] }, + ])( + "attempts every rollback and preserves the original error when $failures restoration fails", + async ({ failures }) => { + const storagePath = await fs.mkdtemp(path.join(os.tmpdir(), "zoo-history-workspace-rollback-failure-")) + const taskDir = path.join(storagePath, "tasks", "task-1602") + const checkpointsDir = path.join(taskDir, "checkpoints") + const messagesPath = path.join(taskDir, "ui_messages.json") + const originalMessages = [ + { type: "say", say: "task", ts: 1, text: "Continue" }, + { type: "say", say: "checkpoint_saved", ts: 2, text: "old-hash" }, + ] + await fs.mkdir(checkpointsDir, { recursive: true }) + await fs.writeFile(path.join(checkpointsDir, "HEAD"), "old checkpoint") + await fs.writeFile(messagesPath, JSON.stringify(originalMessages)) + + const provider = createProvider("/current/workspace") + const original = historyItem("/old/worktree") + const updated = { ...original, workspace: "/current/workspace" } + Object.defineProperty(provider, "contextProxy", { + value: { globalStorageUri: { fsPath: storagePath } }, + }) + // History can already be committed when broadcasting the update fails. + Object.defineProperty(provider, "taskHistoryStore", { value: { get: vi.fn().mockReturnValue(updated) } }) + const originalError = new Error("history broadcast failed") + const updateHistory = vi.fn().mockRejectedValueOnce(originalError) + if (failures.includes("history")) { + updateHistory.mockRejectedValueOnce(new Error("history restore failed")) + } else { + updateHistory.mockResolvedValueOnce([original]) + } + provider.updateTaskHistory = updateHistory + provider["log"] = vi.fn() + const saveMessages = taskMessages.saveTaskMessages + const save = vi.spyOn(taskMessages, "saveTaskMessages").mockImplementationOnce(saveMessages) + if (failures.includes("messages")) { + save.mockRejectedValueOnce(new Error("messages restore failed")) + } + const rename = fs.rename + const renameSpy = vi.spyOn(fs, "rename").mockImplementation(async (source, destination) => { + if (destination === checkpointsDir && failures.includes("checkpoints")) { + throw new Error("checkpoints restore failed") + } + await rename(source, destination) + }) + + try { + await expect(provider["resetTaskCheckpointsForWorkspaceChange"](original, updated)).rejects.toBe( + originalError, + ) + expect(save).toHaveBeenCalledTimes(2) + expect(save.mock.calls[1][0]).toMatchObject({ + messages: originalMessages, + taskId: original.id, + globalStoragePath: storagePath, + }) + expect(renameSpy).toHaveBeenCalledWith( + expect.stringContaining("checkpoints.workspace-change-"), + checkpointsDir, + ) + expect(updateHistory).toHaveBeenCalledTimes(2) + expect(updateHistory).toHaveBeenLastCalledWith(original) + expect(provider["log"]).toHaveBeenCalledTimes(failures.length) + for (const failure of failures) { + expect(provider["log"]).toHaveBeenCalledWith(expect.stringContaining(`${failure} restore failed`)) + } + if (!failures.includes("messages")) { + expect(JSON.parse(await fs.readFile(messagesPath, "utf8"))).toMatchObject(originalMessages) + } + if (!failures.includes("checkpoints")) { + await expect(fs.readFile(path.join(checkpointsDir, "HEAD"), "utf8")).resolves.toBe("old checkpoint") + } + } finally { + await fs.rm(storagePath, { recursive: true, force: true }) + } + }, + ) + it("keeps committed history when checkpoint backup cleanup fails", async () => { const storagePath = await fs.mkdtemp(path.join(os.tmpdir(), "zoo-history-workspace-cleanup-")) const taskDir = path.join(storagePath, "tasks", "task-1602") From 31822b1f133d3d367d775e8f93e169ce39ee9f43 Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Mon, 5 Oct 2026 19:05:50 +0200 Subject: [PATCH 6/8] fix(history): preserve concurrent messages during workspace reset Amp-Thread-ID: https://ampcode.com/threads/T-01a10cc3-928d-7028-bd4b-83dcc6816f1b Co-authored-by: Amp --- apps/vscode-e2e/src/visual/electron.visual.ts | 5 -- .../__tests__/taskMessages.spec.ts | 51 ++++++++++- src/core/task-persistence/taskMessages.ts | 18 ++++ src/core/webview/ClineProvider.ts | 41 +++++---- .../ClineProvider.history-workspace.spec.ts | 90 ++++++++++++++++--- .../__tests__/api-resume-task.spec.ts | 16 +++- 6 files changed, 185 insertions(+), 36 deletions(-) diff --git a/apps/vscode-e2e/src/visual/electron.visual.ts b/apps/vscode-e2e/src/visual/electron.visual.ts index affeb9b46d..155f496980 100644 --- a/apps/vscode-e2e/src/visual/electron.visual.ts +++ b/apps/vscode-e2e/src/visual/electron.visual.ts @@ -122,8 +122,6 @@ async function startScene( mock.addFixture({ match: { userMessage: CHAT_PROMPT }, response: { - // Keep the context meter independent of system prompts and temporary workspace paths. - usage: { prompt_tokens: 4096, completion_tokens: 128, total_tokens: 4224 }, toolCalls: [ { name: "attempt_completion", @@ -232,9 +230,6 @@ for (const scenario of scenarios) { await contentFrame .getByText(scenario.landmark, { exact: false }) .waitFor({ state: "visible", timeout: 60_000 }) - if (scenario.scene === "chat") { - await expect(contentFrame.getByTestId("context-tokens-count")).toHaveText("4.2k") - } await contentFrame.evaluate(() => { if (document.activeElement instanceof HTMLElement) document.activeElement.blur() }) diff --git a/src/core/task-persistence/__tests__/taskMessages.spec.ts b/src/core/task-persistence/__tests__/taskMessages.spec.ts index 6b968fe96c..95f4602950 100644 --- a/src/core/task-persistence/__tests__/taskMessages.spec.ts +++ b/src/core/task-persistence/__tests__/taskMessages.spec.ts @@ -19,7 +19,7 @@ vi.mock("../../../utils/safeWriteJson", () => ({ })) // Import after mocks -import { saveTaskMessages, readTaskMessages } from "../taskMessages" +import { saveTaskMessages, readTaskMessages, updateTaskMessages } from "../taskMessages" import { formatErrorDetail } from "../readFileWithMissingRetry" let tmpBaseDir: string @@ -119,6 +119,55 @@ describe("taskMessages.saveTaskMessages", () => { }) }) +describe("taskMessages.updateTaskMessages", () => { + it("keeps legacy same-timestamp identities distinct across removal and rollback", async () => { + const { safeWriteJson } = + await vi.importActual("../../../utils/safeWriteJson") + const options = { taskId: "task-legacy-update", globalStoragePath: tmpBaseDir } + hoisted.safeWriteJsonMock.mockImplementationOnce(safeWriteJson) + await saveTaskMessages({ ...options, messages: [] }) + await fs.writeFile( + path.join(tmpBaseDir, "tasks", options.taskId, "ui_messages.json"), + JSON.stringify([ + { ts: 1, type: "say", say: "checkpoint_saved", text: "checkpoint" }, + { ts: 1, type: "say", say: "text", text: "keep" }, + ]), + ) + let removed: ClineMessage[] = [] + hoisted.safeWriteJsonMock.mockImplementationOnce(safeWriteJson) + await updateTaskMessages({ + ...options, + update: (messages) => { + removed = messages.slice(0, 1) + return messages.slice(1) + }, + }) + hoisted.safeWriteJsonMock.mockImplementationOnce(safeWriteJson) + await saveTaskMessages({ ...options, messages: removed, merge: true }) + const messages = await readTaskMessages(options) + expect(messages.map((message) => message.text).sort()).toEqual(["checkpoint", "keep"]) + expect(new Set(messages.map((message) => message.messageId)).size).toBe(2) + }) + + it.each([null, "invalid", {}])("rejects a missing or invalid locked snapshot: %j", async (existing: unknown) => { + const { safeWriteJson } = + await vi.importActual("../../../utils/safeWriteJson") + hoisted.safeWriteJsonMock.mockImplementationOnce(safeWriteJson) + const taskId = "task-invalid-update" + const taskDir = path.join(tmpBaseDir, "tasks", taskId) + await fs.mkdir(taskDir, { recursive: true }) + const filePath = path.join(taskDir, "ui_messages.json") + if (existing !== null) await fs.writeFile(filePath, JSON.stringify(existing)) + const update = vi.fn((messages: ClineMessage[]) => messages) + await expect(updateTaskMessages({ taskId, globalStoragePath: tmpBaseDir, update })).rejects.toMatchObject({ + kind: "invalid", + }) + expect(update).not.toHaveBeenCalled() + if (existing !== null) expect(JSON.parse(await fs.readFile(filePath, "utf8"))).toEqual(existing) + else await expect(fs.stat(filePath)).rejects.toMatchObject({ code: "ENOENT" }) + }) +}) + describe("taskMessages.readTaskMessages", () => { it("rejects invalid JSON without treating it as empty history", async () => { const taskId = "task-corrupt-json" diff --git a/src/core/task-persistence/taskMessages.ts b/src/core/task-persistence/taskMessages.ts index b67ad2b7f8..961c1790f0 100644 --- a/src/core/task-persistence/taskMessages.ts +++ b/src/core/task-persistence/taskMessages.ts @@ -73,6 +73,24 @@ export type SaveTaskMessagesOptions = { merge?: boolean } +/** Update the current disk snapshot under the same lock used by every message writer. */ +export async function updateTaskMessages({ + taskId, + globalStoragePath, + update, +}: ReadTaskMessagesOptions & { update: (messages: ClineMessage[]) => ClineMessage[] }): Promise { + const taskDir = await getTaskDirectoryPath(globalStoragePath, taskId) + const filePath = path.join(taskDir, GlobalFileNames.uiMessages) + await safeWriteJson(filePath, null, { + merge: (existing) => { + if (!Array.isArray(existing)) { + throw new TaskMessagesReadError("invalid", `Cannot update missing or invalid messages for ${taskId}`) + } + return ensureMessageIdentifiers(update(ensureMessageIdentifiers(existing))) + }, + }) +} + export async function saveTaskMessages({ messages, taskId, diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 66a3021130..09fde9c1bd 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -132,7 +132,7 @@ import { interruptDelegatedChild, LifecycleTransitionError, } from "../task-persistence" -import { readTaskMessages } from "../task-persistence/taskMessages" +import { readTaskMessages, updateTaskMessages } from "../task-persistence/taskMessages" import { getNonce } from "./getNonce" import { getUri } from "./getUri" import { omitOriginalContentFromExtensionMessage } from "./stripOriginalContent" @@ -2481,10 +2481,7 @@ export class ClineProvider ): Promise { const taskId = originalHistoryItem.id const globalStoragePath = this.contextProxy.globalStorageUri.fsPath - const messages = await readTaskMessages({ taskId, globalStoragePath }) - const messagesWithoutCheckpoints = messages.filter( - (message) => !(message.type === "say" && message.say === "checkpoint_saved"), - ) + let removedCheckpoints: ClineMessage[] = [] const taskDir = await getTaskDirectoryPath(globalStoragePath, taskId) const checkpointsDir = path.join(taskDir, "checkpoints") const checkpointBackupDir = path.join(taskDir, `checkpoints.workspace-change-${crypto.randomUUID()}`) @@ -2500,14 +2497,22 @@ export class ClineProvider } try { - if (messagesWithoutCheckpoints.length !== messages.length) { - await saveTaskMessages({ messages: messagesWithoutCheckpoints, taskId, globalStoragePath }) - } + await updateTaskMessages({ + taskId, + globalStoragePath, + update: (messages) => { + removedCheckpoints = messages.filter( + (message) => message.type === "say" && message.say === "checkpoint_saved", + ) + return messages.filter((message) => !removedCheckpoints.includes(message)) + }, + }) await this.updateTaskHistory(updatedHistoryItem) } catch (error) { - if (messagesWithoutCheckpoints.length !== messages.length) { + if (removedCheckpoints.length > 0) { try { - await saveTaskMessages({ messages, taskId, globalStoragePath }) + // Restore only the removed rows; never replace intervening message writes. + await saveTaskMessages({ messages: removedCheckpoints, taskId, globalStoragePath, merge: true }) } catch (rollbackError) { this.log( `[resetTaskCheckpointsForWorkspaceChange] Failed to restore messages for ${taskId}: ${rollbackError instanceof Error ? rollbackError.message : String(rollbackError)}`, @@ -2536,12 +2541,16 @@ export class ClineProvider } if (checkpointDirectoryStaged) { - try { - await fs.rm(checkpointBackupDir, { recursive: true, force: true }) - } catch (error) { - this.log( - `[resetTaskCheckpointsForWorkspaceChange] Failed to remove checkpoint backup for ${taskId}: ${error instanceof Error ? error.message : String(error)}`, - ) + for (let attempt = 0; attempt < 3; attempt++) { + try { + await fs.rm(checkpointBackupDir, { recursive: true, force: true }) + break + } catch (error) { + this.log( + `[resetTaskCheckpointsForWorkspaceChange] Failed to remove checkpoint backup ${checkpointBackupDir}: ${error instanceof Error ? error.message : String(error)}`, + ) + if (attempt < 2) await delay(100) + } } } } diff --git a/src/core/webview/__tests__/ClineProvider.history-workspace.spec.ts b/src/core/webview/__tests__/ClineProvider.history-workspace.spec.ts index 223ffe3482..4daf9e7b62 100644 --- a/src/core/webview/__tests__/ClineProvider.history-workspace.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.history-workspace.spec.ts @@ -3,7 +3,7 @@ import os from "os" import path from "path" import * as vscode from "vscode" -import type { HistoryItem } from "@roo-code/types" +import type { ClineMessage, HistoryItem } from "@roo-code/types" import { ClineProvider } from "../ClineProvider" import * as taskMessages from "../../task-persistence/taskMessages" @@ -41,6 +41,17 @@ describe("ClineProvider historical workspace selection", () => { expect(prompt).not.toHaveBeenCalled() }) + it.each([ + { current: "", original: "/old/worktree" }, + { current: "/current/workspace", original: undefined }, + ])("does not prompt when a workspace is absent ($current, $original)", async ({ current, original }) => { + const provider = createProvider(current) + const prompt = vi.spyOn(vscode.window, "showWarningMessage") + const item = { ...historyItem("/old/worktree"), workspace: original } + await expect(provider.prepareHistoryItemForResume(item)).resolves.toBe(item) + expect(prompt).not.toHaveBeenCalled() + }) + it("cancels restoration without mutating history when the mismatch prompt is dismissed", async () => { const provider = createProvider("/current/workspace") vi.spyOn(vscode.window, "showWarningMessage").mockResolvedValue(undefined) @@ -124,12 +135,15 @@ describe("ClineProvider historical workspace selection", () => { expect(provider.postMessageToWebview).not.toHaveBeenCalled() }) - it("removes the old checkpoint repository and checkpoint-only chat rows", async () => { + it.each([true, false])("resets checkpoint rows with an existing checkpoint directory: %s", async (hasDirectory) => { const storagePath = await fs.mkdtemp(path.join(os.tmpdir(), "zoo-history-workspace-")) const taskDir = path.join(storagePath, "tasks", "task-1602") const checkpointsDir = path.join(taskDir, "checkpoints") - await fs.mkdir(checkpointsDir, { recursive: true }) - await fs.writeFile(path.join(checkpointsDir, "HEAD"), "old checkpoint") + await fs.mkdir(taskDir, { recursive: true }) + if (hasDirectory) { + await fs.mkdir(checkpointsDir) + await fs.writeFile(path.join(checkpointsDir, "HEAD"), "old checkpoint") + } await fs.writeFile( path.join(taskDir, "ui_messages.json"), JSON.stringify([ @@ -163,6 +177,58 @@ describe("ClineProvider historical workspace selection", () => { } }) + it.each([false, true])("preserves intervening message writes (rollback: %s)", async (rollback) => { + const storagePath = await fs.mkdtemp(path.join(os.tmpdir(), "zoo-history-concurrent-")) + const taskDir = path.join(storagePath, "tasks", "task-1602") + const checkpointsDir = path.join(taskDir, "checkpoints") + await fs.mkdir(checkpointsDir, { recursive: true }) + const original = historyItem("/old/worktree") + const updated = { ...original, workspace: "/current/workspace" } + const options = { taskId: original.id, globalStoragePath: storagePath } + const checkpoint: ClineMessage = { ts: 2, type: "say", say: "checkpoint_saved", text: "old-hash" } + await taskMessages.saveTaskMessages({ + ...options, + messages: [{ ts: 1, type: "say", say: "text", text: "before" }, checkpoint], + }) + const provider = createProvider(updated.workspace) + Object.defineProperty(provider, "contextProxy", { value: { globalStorageUri: { fsPath: storagePath } } }) + Object.defineProperty(provider, "taskHistoryStore", { value: { get: () => original } }) + const rename = fs.rename + vi.spyOn(fs, "rename").mockImplementation(async (source, destination) => { + await rename(source, destination) + if (source === checkpointsDir) { + // A live task saves after reset starts, before the locked removal. + await taskMessages.saveTaskMessages({ + ...options, + merge: true, + messages: [{ ts: 3, type: "say", say: "text", text: "during reset" }], + }) + } + }) + provider.updateTaskHistory = vi.fn().mockImplementation(async () => { + await taskMessages.saveTaskMessages({ + ...options, + merge: true, + messages: [{ ts: 1, type: "say", say: "text", text: "updated during history write" }], + }) + if (rollback) throw new Error("history write failed") + return [updated] + }) + try { + const reset = provider["resetTaskCheckpointsForWorkspaceChange"](original, updated) + if (rollback) await expect(reset).rejects.toThrow("history write failed") + else await reset + const messages = await taskMessages.readTaskMessages(options) + expect(messages.map(({ ts, text }) => ({ ts, text }))).toEqual([ + { ts: 1, text: "updated during history write" }, + ...(rollback ? [{ ts: 2, text: "old-hash" }] : []), + { ts: 3, text: "during reset" }, + ]) + } finally { + await fs.rm(storagePath, { recursive: true, force: true }) + } + }) + it("restores checkpoint files and messages when workspace persistence fails", async () => { const storagePath = await fs.mkdtemp(path.join(os.tmpdir(), "zoo-history-workspace-rollback-")) const taskDir = path.join(storagePath, "tasks", "task-1602") @@ -233,8 +299,7 @@ describe("ClineProvider historical workspace selection", () => { } provider.updateTaskHistory = updateHistory provider["log"] = vi.fn() - const saveMessages = taskMessages.saveTaskMessages - const save = vi.spyOn(taskMessages, "saveTaskMessages").mockImplementationOnce(saveMessages) + const save = vi.spyOn(taskMessages, "saveTaskMessages") if (failures.includes("messages")) { save.mockRejectedValueOnce(new Error("messages restore failed")) } @@ -250,11 +315,12 @@ describe("ClineProvider historical workspace selection", () => { await expect(provider["resetTaskCheckpointsForWorkspaceChange"](original, updated)).rejects.toBe( originalError, ) - expect(save).toHaveBeenCalledTimes(2) - expect(save.mock.calls[1][0]).toMatchObject({ - messages: originalMessages, + expect(save).toHaveBeenCalledOnce() + expect(save.mock.calls[0][0]).toMatchObject({ + messages: [originalMessages[1]], taskId: original.id, globalStoragePath: storagePath, + merge: true, }) expect(renameSpy).toHaveBeenCalledWith( expect.stringContaining("checkpoints.workspace-change-"), @@ -278,7 +344,7 @@ describe("ClineProvider historical workspace selection", () => { }, ) - it("keeps committed history when checkpoint backup cleanup fails", async () => { + it("retries checkpoint backup cleanup without rolling back committed history", async () => { const storagePath = await fs.mkdtemp(path.join(os.tmpdir(), "zoo-history-workspace-cleanup-")) const taskDir = path.join(storagePath, "tasks", "task-1602") const checkpointsDir = path.join(taskDir, "checkpoints") @@ -310,6 +376,10 @@ describe("ClineProvider historical workspace selection", () => { expect(provider.updateTaskHistory).toHaveBeenCalledOnce() expect(provider.updateTaskHistory).toHaveBeenCalledWith(updated) expect(provider["log"]).toHaveBeenCalledWith(expect.stringContaining("backup cleanup failed")) + expect(vi.mocked(fs.rm)).toHaveBeenCalledTimes(2) + expect( + (await fs.readdir(taskDir)).filter((name) => name.startsWith("checkpoints.workspace-change-")), + ).toEqual([]) await expect(fs.stat(checkpointsDir)).rejects.toMatchObject({ code: "ENOENT" }) expect(JSON.parse(await fs.readFile(messagesPath, "utf8"))).toMatchObject([ { type: "say", say: "task", text: "Continue" }, diff --git a/src/extension/__tests__/api-resume-task.spec.ts b/src/extension/__tests__/api-resume-task.spec.ts index 8c4b5043ef..3dac9d35d0 100644 --- a/src/extension/__tests__/api-resume-task.spec.ts +++ b/src/extension/__tests__/api-resume-task.spec.ts @@ -7,27 +7,35 @@ import type { ClineProvider } from "../../core/webview/ClineProvider" const historyItem = { id: "task-1602", + number: 1, + ts: 1, task: "Resume task", tokensIn: 0, tokensOut: 0, totalCost: 0, -} as HistoryItem +} satisfies HistoryItem describe("API.resumeTask", () => { - it("does not restore a task when workspace selection is cancelled", async () => { + it.each([false, true])("resumes only with the prepared history item (accepted: %s)", async (accepted) => { + const prepared = accepted ? { ...historyItem, workspace: "/selected/workspace" } : undefined + // API uses only this provider surface; constructing a full provider starts unrelated services. const provider = { viewLaunched: true, context: {}, getTaskWithId: vi.fn().mockResolvedValue({ historyItem }), - prepareHistoryItemForResume: vi.fn().mockResolvedValue(undefined), + prepareHistoryItemForResume: vi.fn().mockResolvedValue(prepared), createTaskWithHistoryItem: vi.fn(), + postMessageToWebview: vi.fn().mockResolvedValue(true), on: vi.fn(), } as unknown as ClineProvider + // Only appendLine is used by this API test. const outputChannel = { appendLine: vi.fn() } as unknown as vscode.OutputChannel const api = new API(outputChannel, provider) await api.resumeTask(historyItem.id) - expect(provider.createTaskWithHistoryItem).not.toHaveBeenCalled() + expect(provider.prepareHistoryItemForResume).toHaveBeenCalledWith(historyItem) + if (accepted) expect(provider.createTaskWithHistoryItem).toHaveBeenCalledExactlyOnceWith(prepared) + else expect(provider.createTaskWithHistoryItem).not.toHaveBeenCalled() }) }) From 273309858355e63ec5dcd0ccf21e8bb58a5a6f5a Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Mon, 19 Oct 2026 20:20:53 +0200 Subject: [PATCH 7/8] fix(history): update only the workspace when moving a resumed task The workspace-mismatch prompt has no timeout, so the history item read before it can be stale by the time the user chooses. Writing that full snapshot back (and the original snapshot on rollback) could overwrite a newer status or pendingAction through the store's delta merge. Persist and roll back only `workspace` with atomicReadAndUpdate on the current record, roll back only if no newer write moved the workspace again, and resume from the record as persisted rather than the snapshot. Skip the prompt and reset when any provider holds a live Task for the id, and cancel if the task becomes live while the prompt is open, so a live task cannot re-add stripped checkpoint rows or rewrite workspace. This covers API resumes of the current task as well as showTaskWithId. Move the reset specs onto a real TaskHistoryStore so the delta merge is exercised, including a field changed while the prompt is open. Co-Authored-By: Claude Opus 5.5 --- src/core/webview/ClineProvider.ts | 68 ++- .../ClineProvider.history-workspace.spec.ts | 459 ++++++++++-------- 2 files changed, 326 insertions(+), 201 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 09fde9c1bd..75c0aa2aaf 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -2449,6 +2449,11 @@ export class ClineProvider return historyItem } + // A live Task owns this history and would re-add checkpoint rows or rewrite its workspace after a reset. + if (this.isTaskLiveInAnyProvider(historyItem.id)) { + return historyItem + } + const useCurrentWorkspace = { title: "Use Current Workspace" } const openOriginalWorkspace = { title: "Open Original Workspace" } const selection = await vscode.window.showWarningMessage( @@ -2470,18 +2475,62 @@ export class ClineProvider return undefined } + // The modal has no timeout, so the task may have been opened elsewhere while it was shown. + if (this.isTaskLiveInAnyProvider(historyItem.id)) { + this.log(`[prepareHistoryItemForResume] Task ${historyItem.id} became live during workspace selection`) + return undefined + } + const updatedHistoryItem = { ...historyItem, workspace: currentWorkspace } - await this.resetTaskCheckpointsForWorkspaceChange(historyItem, updatedHistoryItem) - return updatedHistoryItem + const persisted = await this.resetTaskCheckpointsForWorkspaceChange(historyItem, updatedHistoryItem) + // Resume from the record as persisted now, not the snapshot read before the prompt. + return { ...historyItem, ...persisted, workspace: currentWorkspace } + } + + private isTaskLiveInAnyProvider(taskId: string): boolean { + return ClineProvider.getAllInstances().some((provider) => provider.taskRegistry.getById(taskId) !== undefined) + } + + /** + * Persist only `workspace`, read-modify-writing the stored record so fields that changed meanwhile + * (status, pendingAction, ...) are never overwritten from a stale snapshot. With `expectedWorkspace`, + * the stored record is left unchanged unless it still has that workspace. + */ + private async setTaskHistoryWorkspace( + historyItem: HistoryItem, + workspace: string | undefined, + expectedWorkspace?: string, + ): Promise { + const taskId = historyItem.id + if (!this.taskHistoryStore.get(taskId)) { + // Legacy fallback before the store is initialized: there is no stored record to preserve. + if (expectedWorkspace !== undefined) return undefined + await this.updateTaskHistory({ ...historyItem, workspace }) + return this.taskHistoryStore.get(taskId) + } + + let changed = false + await this.taskHistoryStore.atomicReadAndUpdate(taskId, (current) => { + if (expectedWorkspace !== undefined && current.workspace !== expectedWorkspace) return current + changed = current.workspace !== workspace + return { ...current, workspace } + }) + this.recentTasksCache = undefined + const updated = this.taskHistoryStore.get(taskId) + if (changed && updated && this.isViewLaunched) { + await this.postMessageToWebview({ type: "taskHistoryItemUpdated", taskHistoryItem: updated }) + } + return updated } private async resetTaskCheckpointsForWorkspaceChange( originalHistoryItem: HistoryItem, updatedHistoryItem: HistoryItem, - ): Promise { + ): Promise { const taskId = originalHistoryItem.id const globalStoragePath = this.contextProxy.globalStorageUri.fsPath let removedCheckpoints: ClineMessage[] = [] + let persisted: HistoryItem | undefined const taskDir = await getTaskDirectoryPath(globalStoragePath, taskId) const checkpointsDir = path.join(taskDir, "checkpoints") const checkpointBackupDir = path.join(taskDir, `checkpoints.workspace-change-${crypto.randomUUID()}`) @@ -2507,7 +2556,7 @@ export class ClineProvider return messages.filter((message) => !removedCheckpoints.includes(message)) }, }) - await this.updateTaskHistory(updatedHistoryItem) + persisted = await this.setTaskHistoryWorkspace(originalHistoryItem, updatedHistoryItem.workspace) } catch (error) { if (removedCheckpoints.length > 0) { try { @@ -2529,9 +2578,12 @@ export class ClineProvider } } try { - if (this.taskHistoryStore.get(taskId)?.workspace === updatedHistoryItem.workspace) { - await this.updateTaskHistory(originalHistoryItem) - } + // Revert only the workspace, and only if no newer write has moved it again. + await this.setTaskHistoryWorkspace( + originalHistoryItem, + originalHistoryItem.workspace, + updatedHistoryItem.workspace, + ) } catch (rollbackError) { this.log( `[resetTaskCheckpointsForWorkspaceChange] Failed to restore history for ${taskId}: ${rollbackError instanceof Error ? rollbackError.message : String(rollbackError)}`, @@ -2553,6 +2605,8 @@ export class ClineProvider } } } + + return persisted } async exportTaskWithId(id: string) { diff --git a/src/core/webview/__tests__/ClineProvider.history-workspace.spec.ts b/src/core/webview/__tests__/ClineProvider.history-workspace.spec.ts index 4daf9e7b62..2343fc3035 100644 --- a/src/core/webview/__tests__/ClineProvider.history-workspace.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.history-workspace.spec.ts @@ -3,10 +3,14 @@ import os from "os" import path from "path" import * as vscode from "vscode" -import type { ClineMessage, HistoryItem } from "@roo-code/types" +import type { HistoryItem, PendingTaskAction } from "@roo-code/types" import { ClineProvider } from "../ClineProvider" +import type { Task } from "../../task/Task" +import { TaskRegistry } from "../../task/TaskRegistry" +import { TaskHistoryStore } from "../../task-persistence/TaskHistoryStore" import * as taskMessages from "../../task-persistence/taskMessages" +import { GlobalFileNames } from "../../../shared/globalFileNames" const historyItem = (workspace: string): HistoryItem => ({ id: "task-1602", @@ -135,186 +139,218 @@ describe("ClineProvider historical workspace selection", () => { expect(provider.postMessageToWebview).not.toHaveBeenCalled() }) - it.each([true, false])("resets checkpoint rows with an existing checkpoint directory: %s", async (hasDirectory) => { - const storagePath = await fs.mkdtemp(path.join(os.tmpdir(), "zoo-history-workspace-")) - const taskDir = path.join(storagePath, "tasks", "task-1602") - const checkpointsDir = path.join(taskDir, "checkpoints") - await fs.mkdir(taskDir, { recursive: true }) - if (hasDirectory) { - await fs.mkdir(checkpointsDir) - await fs.writeFile(path.join(checkpointsDir, "HEAD"), "old checkpoint") - } - await fs.writeFile( - path.join(taskDir, "ui_messages.json"), - JSON.stringify([ - { type: "say", say: "task", ts: 1, text: "Continue" }, - { type: "say", say: "checkpoint_saved", ts: 2, text: "old-hash" }, - { type: "say", say: "text", ts: 3, text: "Still useful" }, - ]), - ) + describe("with a persisted task history store", () => { + let storagePath: string + let taskDir: string + let checkpointsDir: string + let messagesPath: string + let store: TaskHistoryStore - const provider = createProvider("/current/workspace") - Object.defineProperty(provider, "contextProxy", { - value: { globalStorageUri: { fsPath: storagePath } }, - }) const original = historyItem("/old/worktree") const updated = { ...original, workspace: "/current/workspace" } - provider.updateTaskHistory = vi.fn().mockResolvedValue([]) - Object.defineProperty(provider, "taskHistoryStore", { value: { get: vi.fn().mockReturnValue(original) } }) - try { - await provider["resetTaskCheckpointsForWorkspaceChange"](original, updated) + beforeEach(async () => { + storagePath = await fs.mkdtemp(path.join(os.tmpdir(), "zoo-history-workspace-")) + taskDir = path.join(storagePath, "tasks", original.id) + checkpointsDir = path.join(taskDir, "checkpoints") + messagesPath = path.join(taskDir, "ui_messages.json") + store = new TaskHistoryStore(storagePath) + await store.initialize() + store.dispose() // Keep watcher reconciliation out of these explicit write tests. + await store.upsert(original) + }) - await expect(fs.stat(checkpointsDir)).rejects.toMatchObject({ code: "ENOENT" }) - const messages = JSON.parse(await fs.readFile(path.join(taskDir, "ui_messages.json"), "utf8")) - expect(messages).toMatchObject([ - { type: "say", say: "task", text: "Continue" }, - { type: "say", say: "text", text: "Still useful" }, - ]) - expect(provider.updateTaskHistory).toHaveBeenCalledWith(updated) - } finally { + afterEach(async () => { + ClineProvider["activeInstances"].clear() await fs.rm(storagePath, { recursive: true, force: true }) + }) + + const originalMessages = [ + { type: "say", say: "task", ts: 1, text: "Continue" }, + { type: "say", say: "checkpoint_saved", ts: 2, text: "old-hash" }, + ] + + const createStoredProvider = () => { + const provider = createProvider("/current/workspace") + Object.defineProperty(provider, "contextProxy", { value: { globalStorageUri: { fsPath: storagePath } } }) + Object.defineProperty(provider, "taskHistoryStore", { value: store }) + provider["log"] = vi.fn() + return provider } - }) - it.each([false, true])("preserves intervening message writes (rollback: %s)", async (rollback) => { - const storagePath = await fs.mkdtemp(path.join(os.tmpdir(), "zoo-history-concurrent-")) - const taskDir = path.join(storagePath, "tasks", "task-1602") - const checkpointsDir = path.join(taskDir, "checkpoints") - await fs.mkdir(checkpointsDir, { recursive: true }) - const original = historyItem("/old/worktree") - const updated = { ...original, workspace: "/current/workspace" } - const options = { taskId: original.id, globalStoragePath: storagePath } - const checkpoint: ClineMessage = { ts: 2, type: "say", say: "checkpoint_saved", text: "old-hash" } - await taskMessages.saveTaskMessages({ - ...options, - messages: [{ ts: 1, type: "say", say: "text", text: "before" }, checkpoint], - }) - const provider = createProvider(updated.workspace) - Object.defineProperty(provider, "contextProxy", { value: { globalStorageUri: { fsPath: storagePath } } }) - Object.defineProperty(provider, "taskHistoryStore", { value: { get: () => original } }) - const rename = fs.rename - vi.spyOn(fs, "rename").mockImplementation(async (source, destination) => { - await rename(source, destination) - if (source === checkpointsDir) { - // A live task saves after reset starts, before the locked removal. + const writeCheckpointState = async () => { + await fs.mkdir(checkpointsDir, { recursive: true }) + await fs.writeFile(path.join(checkpointsDir, "HEAD"), "old checkpoint") + await fs.writeFile(messagesPath, JSON.stringify(originalMessages)) + } + + const readPersistedHistory = async () => + JSON.parse(await fs.readFile(path.join(taskDir, GlobalFileNames.historyItem), "utf8")) as HistoryItem + + it.each([true, false])( + "resets checkpoint rows with an existing checkpoint directory: %s", + async (hasDirectory) => { + if (hasDirectory) { + await fs.mkdir(checkpointsDir) + await fs.writeFile(path.join(checkpointsDir, "HEAD"), "old checkpoint") + } + await fs.writeFile( + messagesPath, + JSON.stringify([ + { type: "say", say: "task", ts: 1, text: "Continue" }, + { type: "say", say: "checkpoint_saved", ts: 2, text: "old-hash" }, + { type: "say", say: "text", ts: 3, text: "Still useful" }, + ]), + ) + const provider = createStoredProvider() + + await expect(provider["resetTaskCheckpointsForWorkspaceChange"](original, updated)).resolves.toEqual( + updated, + ) + + await expect(fs.stat(checkpointsDir)).rejects.toMatchObject({ code: "ENOENT" }) + expect(JSON.parse(await fs.readFile(messagesPath, "utf8"))).toMatchObject([ + { type: "say", say: "task", text: "Continue" }, + { type: "say", say: "text", text: "Still useful" }, + ]) + expect(store.get(original.id)?.workspace).toBe("/current/workspace") + expect((await readPersistedHistory()).workspace).toBe("/current/workspace") + }, + ) + + it.each([false, true])( + "keeps history fields written while the prompt was open (rollback: %s)", + async (rollback) => { + await writeCheckpointState() + const pendingAction: PendingTaskAction = { + kind: "create_subtask", + actionId: "action-1", + approvalText: "Create a subtask", + mode: "code", + message: "Do the work", + todos: [], + } + // An explicit stale status is what a full-item write would put back over the newer one. + const snapshot: HistoryItem = { ...original, status: "active" } + await store.upsert(snapshot) + const fresh = { status: "delegated", awaitingChildId: "child-1", pendingAction } as const + const provider = createStoredProvider() + // The stale snapshot is the record read before the modal; another writer updates it meanwhile. + vi.spyOn(vscode.window, "showWarningMessage").mockImplementation(async () => { + await store.atomicReadAndUpdate(original.id, (current) => ({ ...current, ...fresh })) + return { title: "Use Current Workspace" } + }) + const historyBroadcastFailure = new Error("history broadcast failed") + provider.isViewLaunched = rollback + provider.postMessageToWebview = vi.fn().mockRejectedValueOnce(historyBroadcastFailure) + + const prepared = provider.prepareHistoryItemForResume(snapshot) + + const expected = { ...snapshot, ...fresh, workspace: rollback ? original.workspace : updated.workspace } + if (rollback) await expect(prepared).rejects.toBe(historyBroadcastFailure) + else await expect(prepared).resolves.toEqual(expected) + expect(store.get(original.id)).toEqual(expected) + expect(await readPersistedHistory()).toEqual(expected) + }, + ) + + it.each([false, true])("preserves intervening message writes (rollback: %s)", async (rollback) => { + await fs.mkdir(checkpointsDir, { recursive: true }) + const options = { taskId: original.id, globalStoragePath: storagePath } + await taskMessages.saveTaskMessages({ + ...options, + messages: [ + { ts: 1, type: "say", say: "text", text: "before" }, + { ts: 2, type: "say", say: "checkpoint_saved", text: "old-hash" }, + ], + }) + const provider = createStoredProvider() + const rename = fs.rename + vi.spyOn(fs, "rename").mockImplementation(async (source, destination) => { + await rename(source, destination) + if (source === checkpointsDir) { + // A live task saves after reset starts, before the locked removal. + await taskMessages.saveTaskMessages({ + ...options, + merge: true, + messages: [{ ts: 3, type: "say", say: "text", text: "during reset" }], + }) + } + }) + const realAtomicUpdate = store.atomicReadAndUpdate.bind(store) + vi.spyOn(store, "atomicReadAndUpdate").mockImplementationOnce(async (...args) => { await taskMessages.saveTaskMessages({ ...options, merge: true, - messages: [{ ts: 3, type: "say", say: "text", text: "during reset" }], + messages: [{ ts: 1, type: "say", say: "text", text: "updated during history write" }], }) - } - }) - provider.updateTaskHistory = vi.fn().mockImplementation(async () => { - await taskMessages.saveTaskMessages({ - ...options, - merge: true, - messages: [{ ts: 1, type: "say", say: "text", text: "updated during history write" }], + if (rollback) throw new Error("history write failed") + return realAtomicUpdate(...args) }) - if (rollback) throw new Error("history write failed") - return [updated] - }) - try { + const reset = provider["resetTaskCheckpointsForWorkspaceChange"](original, updated) if (rollback) await expect(reset).rejects.toThrow("history write failed") else await reset + const messages = await taskMessages.readTaskMessages(options) expect(messages.map(({ ts, text }) => ({ ts, text }))).toEqual([ { ts: 1, text: "updated during history write" }, ...(rollback ? [{ ts: 2, text: "old-hash" }] : []), { ts: 3, text: "during reset" }, ]) - } finally { - await fs.rm(storagePath, { recursive: true, force: true }) - } - }) - - it("restores checkpoint files and messages when workspace persistence fails", async () => { - const storagePath = await fs.mkdtemp(path.join(os.tmpdir(), "zoo-history-workspace-rollback-")) - const taskDir = path.join(storagePath, "tasks", "task-1602") - const checkpointsDir = path.join(taskDir, "checkpoints") - const messagesPath = path.join(taskDir, "ui_messages.json") - const originalMessages = [ - { type: "say", say: "task", ts: 1, text: "Continue" }, - { type: "say", say: "checkpoint_saved", ts: 2, text: "old-hash" }, - ] - await fs.mkdir(checkpointsDir, { recursive: true }) - await fs.writeFile(path.join(checkpointsDir, "HEAD"), "old checkpoint") - await fs.writeFile(messagesPath, JSON.stringify(originalMessages)) - - const provider = createProvider("/current/workspace") - const original = historyItem("/old/worktree") - const updated = { ...original, workspace: "/current/workspace" } - Object.defineProperty(provider, "contextProxy", { - value: { globalStorageUri: { fsPath: storagePath } }, }) - Object.defineProperty(provider, "taskHistoryStore", { value: { get: vi.fn().mockReturnValue(original) } }) - provider.updateTaskHistory = vi.fn().mockRejectedValue(new Error("history write failed")) - try { + it("restores checkpoint files and messages when workspace persistence fails", async () => { + await writeCheckpointState() + const provider = createStoredProvider() + vi.spyOn(store, "atomicReadAndUpdate").mockRejectedValueOnce(new Error("history write failed")) + await expect(provider["resetTaskCheckpointsForWorkspaceChange"](original, updated)).rejects.toThrow( "history write failed", ) + await expect(fs.readFile(path.join(checkpointsDir, "HEAD"), "utf8")).resolves.toBe("old checkpoint") expect(JSON.parse(await fs.readFile(messagesPath, "utf8"))).toMatchObject(originalMessages) - } finally { - await fs.rm(storagePath, { recursive: true, force: true }) - } - }) - - it.each([ - { failures: ["messages"] }, - { failures: ["checkpoints"] }, - { failures: ["history"] }, - { failures: ["messages", "checkpoints", "history"] }, - ])( - "attempts every rollback and preserves the original error when $failures restoration fails", - async ({ failures }) => { - const storagePath = await fs.mkdtemp(path.join(os.tmpdir(), "zoo-history-workspace-rollback-failure-")) - const taskDir = path.join(storagePath, "tasks", "task-1602") - const checkpointsDir = path.join(taskDir, "checkpoints") - const messagesPath = path.join(taskDir, "ui_messages.json") - const originalMessages = [ - { type: "say", say: "task", ts: 1, text: "Continue" }, - { type: "say", say: "checkpoint_saved", ts: 2, text: "old-hash" }, - ] - await fs.mkdir(checkpointsDir, { recursive: true }) - await fs.writeFile(path.join(checkpointsDir, "HEAD"), "old checkpoint") - await fs.writeFile(messagesPath, JSON.stringify(originalMessages)) + expect(store.get(original.id)?.workspace).toBe("/old/worktree") + }) - const provider = createProvider("/current/workspace") - const original = historyItem("/old/worktree") - const updated = { ...original, workspace: "/current/workspace" } - Object.defineProperty(provider, "contextProxy", { - value: { globalStorageUri: { fsPath: storagePath } }, - }) - // History can already be committed when broadcasting the update fails. - Object.defineProperty(provider, "taskHistoryStore", { value: { get: vi.fn().mockReturnValue(updated) } }) - const originalError = new Error("history broadcast failed") - const updateHistory = vi.fn().mockRejectedValueOnce(originalError) - if (failures.includes("history")) { - updateHistory.mockRejectedValueOnce(new Error("history restore failed")) - } else { - updateHistory.mockResolvedValueOnce([original]) - } - provider.updateTaskHistory = updateHistory - provider["log"] = vi.fn() - const save = vi.spyOn(taskMessages, "saveTaskMessages") - if (failures.includes("messages")) { - save.mockRejectedValueOnce(new Error("messages restore failed")) - } - const rename = fs.rename - const renameSpy = vi.spyOn(fs, "rename").mockImplementation(async (source, destination) => { - if (destination === checkpointsDir && failures.includes("checkpoints")) { - throw new Error("checkpoints restore failed") + it.each([ + { failures: ["messages"] }, + { failures: ["checkpoints"] }, + { failures: ["history"] }, + { failures: ["messages", "checkpoints", "history"] }, + ])( + "attempts every rollback and preserves the original error when $failures restoration fails", + async ({ failures }) => { + await writeCheckpointState() + const provider = createStoredProvider() + // History is already committed when broadcasting the update fails. + const originalError = new Error("history broadcast failed") + provider.isViewLaunched = true + provider.postMessageToWebview = vi.fn().mockRejectedValueOnce(originalError).mockResolvedValue(true) + const realAtomicUpdate = store.atomicReadAndUpdate.bind(store) + const atomicUpdate = vi.spyOn(store, "atomicReadAndUpdate") + if (failures.includes("history")) { + atomicUpdate + .mockImplementationOnce(realAtomicUpdate) + .mockRejectedValueOnce(new Error("history restore failed")) } - await rename(source, destination) - }) + const save = vi.spyOn(taskMessages, "saveTaskMessages") + if (failures.includes("messages")) { + save.mockRejectedValueOnce(new Error("messages restore failed")) + } + const rename = fs.rename + const renameSpy = vi.spyOn(fs, "rename").mockImplementation(async (source, destination) => { + if (destination === checkpointsDir && failures.includes("checkpoints")) { + throw new Error("checkpoints restore failed") + } + await rename(source, destination) + }) - try { await expect(provider["resetTaskCheckpointsForWorkspaceChange"](original, updated)).rejects.toBe( originalError, ) + expect(save).toHaveBeenCalledOnce() expect(save.mock.calls[0][0]).toMatchObject({ messages: [originalMessages[1]], @@ -326,8 +362,10 @@ describe("ClineProvider historical workspace selection", () => { expect.stringContaining("checkpoints.workspace-change-"), checkpointsDir, ) - expect(updateHistory).toHaveBeenCalledTimes(2) - expect(updateHistory).toHaveBeenLastCalledWith(original) + expect(atomicUpdate).toHaveBeenCalledTimes(2) + expect(store.get(original.id)?.workspace).toBe( + failures.includes("history") ? "/current/workspace" : "/old/worktree", + ) expect(provider["log"]).toHaveBeenCalledTimes(failures.length) for (const failure of failures) { expect(provider["log"]).toHaveBeenCalledWith(expect.stringContaining(`${failure} restore failed`)) @@ -338,55 +376,88 @@ describe("ClineProvider historical workspace selection", () => { if (!failures.includes("checkpoints")) { await expect(fs.readFile(path.join(checkpointsDir, "HEAD"), "utf8")).resolves.toBe("old checkpoint") } + }, + ) + + it("does not roll back a workspace that a newer write already moved", async () => { + await writeCheckpointState() + const provider = createStoredProvider() + const broadcastFailure = new Error("history broadcast failed") + provider.isViewLaunched = true + provider.postMessageToWebview = vi.fn().mockImplementationOnce(async () => { + await store.atomicReadAndUpdate(original.id, (current) => ({ + ...current, + workspace: "/newer/workspace", + })) + throw broadcastFailure + }) + + await expect(provider["resetTaskCheckpointsForWorkspaceChange"](original, updated)).rejects.toBe( + broadcastFailure, + ) + + expect(store.get(original.id)?.workspace).toBe("/newer/workspace") + }) + + it("retries checkpoint backup cleanup without rolling back committed history", async () => { + await writeCheckpointState() + const provider = createStoredProvider() + vi.spyOn(fs, "rm").mockRejectedValueOnce(new Error("backup cleanup failed")) + + try { + await expect(provider["resetTaskCheckpointsForWorkspaceChange"](original, updated)).resolves.toEqual( + updated, + ) + + expect(store.get(original.id)?.workspace).toBe("/current/workspace") + expect(provider["log"]).toHaveBeenCalledWith(expect.stringContaining("backup cleanup failed")) + expect(vi.mocked(fs.rm)).toHaveBeenCalledTimes(2) + expect( + (await fs.readdir(taskDir)).filter((name) => name.startsWith("checkpoints.workspace-change-")), + ).toEqual([]) + await expect(fs.stat(checkpointsDir)).rejects.toMatchObject({ code: "ENOENT" }) + expect(JSON.parse(await fs.readFile(messagesPath, "utf8"))).toMatchObject([ + { type: "say", say: "task", text: "Continue" }, + ]) } finally { - await fs.rm(storagePath, { recursive: true, force: true }) + vi.mocked(fs.rm).mockRestore() } - }, - ) - - it("retries checkpoint backup cleanup without rolling back committed history", async () => { - const storagePath = await fs.mkdtemp(path.join(os.tmpdir(), "zoo-history-workspace-cleanup-")) - const taskDir = path.join(storagePath, "tasks", "task-1602") - const checkpointsDir = path.join(taskDir, "checkpoints") - const messagesPath = path.join(taskDir, "ui_messages.json") - await fs.mkdir(checkpointsDir, { recursive: true }) - await fs.writeFile(path.join(checkpointsDir, "HEAD"), "old checkpoint") - await fs.writeFile( - messagesPath, - JSON.stringify([ - { type: "say", say: "task", ts: 1, text: "Continue" }, - { type: "say", say: "checkpoint_saved", ts: 2, text: "old-hash" }, - ]), - ) + }) - const provider = createProvider("/current/workspace") - const original = historyItem("/old/worktree") - const updated = { ...original, workspace: "/current/workspace" } - Object.defineProperty(provider, "contextProxy", { - value: { globalStorageUri: { fsPath: storagePath } }, + it("leaves a task owned by a live Task untouched without prompting", async () => { + await writeCheckpointState() + const provider = createStoredProvider() + const registry = new TaskRegistry() + // TaskRegistry reads only taskId; a full Task would start unrelated services. + registry.push({ taskId: original.id } as Task) + Object.defineProperty(provider, "taskRegistry", { value: registry }) + ClineProvider["activeInstances"].add(provider) + const prompt = vi.spyOn(vscode.window, "showWarningMessage") + + await expect(provider.prepareHistoryItemForResume(original)).resolves.toBe(original) + + expect(prompt).not.toHaveBeenCalled() + expect(store.get(original.id)?.workspace).toBe("/old/worktree") + await expect(fs.readFile(path.join(checkpointsDir, "HEAD"), "utf8")).resolves.toBe("old checkpoint") + }) + + it("cancels without resetting when the task becomes live while the prompt is open", async () => { + await writeCheckpointState() + const provider = createStoredProvider() + const registry = new TaskRegistry() + Object.defineProperty(provider, "taskRegistry", { value: registry }) + ClineProvider["activeInstances"].add(provider) + vi.spyOn(vscode.window, "showWarningMessage").mockImplementation(async () => { + // TaskRegistry reads only taskId; a full Task would start unrelated services. + registry.push({ taskId: original.id } as Task) + return { title: "Use Current Workspace" } + }) + + await expect(provider.prepareHistoryItemForResume(original)).resolves.toBeUndefined() + + expect(store.get(original.id)?.workspace).toBe("/old/worktree") + await expect(fs.readFile(path.join(checkpointsDir, "HEAD"), "utf8")).resolves.toBe("old checkpoint") + expect(JSON.parse(await fs.readFile(messagesPath, "utf8"))).toMatchObject(originalMessages) }) - Object.defineProperty(provider, "taskHistoryStore", { value: { get: vi.fn().mockReturnValue(updated) } }) - provider.updateTaskHistory = vi.fn().mockResolvedValue([]) - provider["log"] = vi.fn() - vi.spyOn(fs, "rm").mockRejectedValueOnce(new Error("backup cleanup failed")) - - try { - await expect(provider["resetTaskCheckpointsForWorkspaceChange"](original, updated)).resolves.toBeUndefined() - - expect(provider.updateTaskHistory).toHaveBeenCalledOnce() - expect(provider.updateTaskHistory).toHaveBeenCalledWith(updated) - expect(provider["log"]).toHaveBeenCalledWith(expect.stringContaining("backup cleanup failed")) - expect(vi.mocked(fs.rm)).toHaveBeenCalledTimes(2) - expect( - (await fs.readdir(taskDir)).filter((name) => name.startsWith("checkpoints.workspace-change-")), - ).toEqual([]) - await expect(fs.stat(checkpointsDir)).rejects.toMatchObject({ code: "ENOENT" }) - expect(JSON.parse(await fs.readFile(messagesPath, "utf8"))).toMatchObject([ - { type: "say", say: "task", text: "Continue" }, - ]) - } finally { - vi.mocked(fs.rm).mockRestore() - await fs.rm(storagePath, { recursive: true, force: true }) - } }) }) From 16203bab6ee926d27da59cee2dec03d156c32d65 Mon Sep 17 00:00:00 2001 From: PierrunoYT Date: Sun, 25 Oct 2026 11:27:13 +0100 Subject: [PATCH 8/8] fix(history): skip checkpoint cleanup without ui_messages.json and bypass the prompt in the CLI Co-Authored-By: Claude Sonnet 5.5 --- src/core/webview/ClineProvider.ts | 29 ++++--- .../ClineProvider.history-workspace.spec.ts | 78 +++++++++++++++++++ .../__tests__/api-resume-task.spec.ts | 3 +- 3 files changed, 99 insertions(+), 11 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 75c0aa2aaf..f3c8356438 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -2445,6 +2445,11 @@ export class ClineProvider const currentWorkspace = this.cwd const originalWorkspace = historyItem.workspace + // The CLI has no modal to answer, and its host would wait forever for a task that is never created. + if (process.env.ROO_CLI_RUNTIME === "1") { + return historyItem + } + if (!currentWorkspace || !originalWorkspace || arePathsEqual(currentWorkspace, originalWorkspace)) { return historyItem } @@ -2546,16 +2551,20 @@ export class ClineProvider } try { - await updateTaskMessages({ - taskId, - globalStoragePath, - update: (messages) => { - removedCheckpoints = messages.filter( - (message) => message.type === "say" && message.say === "checkpoint_saved", - ) - return messages.filter((message) => !removedCheckpoints.includes(message)) - }, - }) + // History migration can create a record for a task directory that has no ui_messages.json; + // that is an empty set of checkpoint rows, not an error. Invalid existing content still rejects. + if (await fileExistsAtPath(path.join(taskDir, GlobalFileNames.uiMessages))) { + await updateTaskMessages({ + taskId, + globalStoragePath, + update: (messages) => { + removedCheckpoints = messages.filter( + (message) => message.type === "say" && message.say === "checkpoint_saved", + ) + return messages.filter((message) => !removedCheckpoints.includes(message)) + }, + }) + } persisted = await this.setTaskHistoryWorkspace(originalHistoryItem, updatedHistoryItem.workspace) } catch (error) { if (removedCheckpoints.length > 0) { diff --git a/src/core/webview/__tests__/ClineProvider.history-workspace.spec.ts b/src/core/webview/__tests__/ClineProvider.history-workspace.spec.ts index 2343fc3035..369b0d19e4 100644 --- a/src/core/webview/__tests__/ClineProvider.history-workspace.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.history-workspace.spec.ts @@ -34,6 +34,57 @@ const createProvider = (workspace: string) => { describe("ClineProvider historical workspace selection", () => { afterEach(() => { vi.restoreAllMocks() + vi.unstubAllEnvs() + }) + + it("never prompts in the CLI runtime, where the modal cannot be answered", async () => { + vi.stubEnv("ROO_CLI_RUNTIME", "1") + const provider = createProvider("/current/workspace") + const prompt = vi.spyOn(vscode.window, "showWarningMessage") + const item = historyItem("/old/worktree") + + await expect(provider.prepareHistoryItemForResume(item)).resolves.toBe(item) + expect(prompt).not.toHaveBeenCalled() + }) + + describe("setTaskHistoryWorkspace without a stored record", () => { + const createLegacyProvider = (stored: HistoryItem | undefined) => { + const provider = createProvider("/current/workspace") + Object.defineProperty(provider, "taskHistoryStore", { value: { get: vi.fn().mockReturnValue(stored) } }) + provider.updateTaskHistory = vi.fn().mockResolvedValue([]) + provider.postMessageToWebview = vi.fn().mockResolvedValue(true) + return provider + } + + it("leaves history alone when a workspace is expected", async () => { + const provider = createLegacyProvider(undefined) + + await expect( + provider["setTaskHistoryWorkspace"]( + historyItem("/old/worktree"), + "/current/workspace", + "/old/worktree", + ), + ).resolves.toBeUndefined() + + expect(provider.updateTaskHistory).not.toHaveBeenCalled() + expect(provider.postMessageToWebview).not.toHaveBeenCalled() + }) + + it("writes the supplied item and returns the stored record when no workspace is expected", async () => { + const stored = historyItem("/current/workspace") + const provider = createLegacyProvider(undefined) + vi.mocked(provider["taskHistoryStore"].get).mockReturnValueOnce(undefined).mockReturnValue(stored) + + await expect( + provider["setTaskHistoryWorkspace"](historyItem("/old/worktree"), "/current/workspace"), + ).resolves.toBe(stored) + + expect(provider.updateTaskHistory).toHaveBeenCalledWith({ + ...historyItem("/old/worktree"), + workspace: "/current/workspace", + }) + }) }) it("keeps a history item unchanged when it already belongs to the current workspace", async () => { @@ -397,6 +448,33 @@ describe("ClineProvider historical workspace selection", () => { ) expect(store.get(original.id)?.workspace).toBe("/newer/workspace") + // The guarded rollback left the newer workspace alone, so it announced nothing more. + expect(provider.postMessageToWebview).toHaveBeenCalledOnce() + }) + + it("moves the workspace when the task has no ui_messages.json", async () => { + await fs.mkdir(checkpointsDir, { recursive: true }) + await fs.writeFile(path.join(checkpointsDir, "HEAD"), "old checkpoint") + const provider = createStoredProvider() + + await expect(provider["resetTaskCheckpointsForWorkspaceChange"](original, updated)).resolves.toEqual( + updated, + ) + + await expect(fs.stat(messagesPath)).rejects.toMatchObject({ code: "ENOENT" }) + await expect(fs.stat(checkpointsDir)).rejects.toMatchObject({ code: "ENOENT" }) + expect(store.get(original.id)?.workspace).toBe("/current/workspace") + }) + + it("still rejects a ui_messages.json with invalid content and keeps the original workspace", async () => { + await writeCheckpointState() + await fs.writeFile(messagesPath, "not json") + const provider = createStoredProvider() + + await expect(provider["resetTaskCheckpointsForWorkspaceChange"](original, updated)).rejects.toThrow() + + expect(store.get(original.id)?.workspace).toBe("/old/worktree") + await expect(fs.readFile(path.join(checkpointsDir, "HEAD"), "utf8")).resolves.toBe("old checkpoint") }) it("retries checkpoint backup cleanup without rolling back committed history", async () => { diff --git a/src/extension/__tests__/api-resume-task.spec.ts b/src/extension/__tests__/api-resume-task.spec.ts index 3dac9d35d0..8385f319d5 100644 --- a/src/extension/__tests__/api-resume-task.spec.ts +++ b/src/extension/__tests__/api-resume-task.spec.ts @@ -4,6 +4,7 @@ import type { HistoryItem } from "@roo-code/types" import { API } from "../api" import type { ClineProvider } from "../../core/webview/ClineProvider" +import { makeClineProviderFactory } from "../../test-utils/provider" const historyItem = { id: "task-1602", @@ -30,7 +31,7 @@ describe("API.resumeTask", () => { } as unknown as ClineProvider // Only appendLine is used by this API test. const outputChannel = { appendLine: vi.fn() } as unknown as vscode.OutputChannel - const api = new API(outputChannel, provider) + const api = new API(outputChannel, provider, makeClineProviderFactory()) await api.resumeTask(historyItem.id)