Skip to content
51 changes: 50 additions & 1 deletion src/core/task-persistence/__tests__/taskMessages.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<typeof import("../../../utils/safeWriteJson")>("../../../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<typeof import("../../../utils/safeWriteJson")>("../../../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"
Expand Down
18 changes: 18 additions & 0 deletions src/core/task-persistence/taskMessages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,24 @@
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<void> {
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}`)

Check warning on line 87 in src/core/task-persistence/taskMessages.ts

View workflow job for this annotation

GitHub Actions / mutation-diff

Mutation test advisory

src/core/task-persistence/taskMessages.ts:87: Survived StringLiteral mutant (replacement: ``). See the job summary for the complete list and resolution guidance.
}
return ensureMessageIdentifiers(update(ensureMessageIdentifiers(existing)))
},
})
}

export async function saveTaskMessages({
messages,
taskId,
Expand Down
188 changes: 185 additions & 3 deletions src/core/webview/ClineProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,9 @@
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 { getTaskDirectoryPath } from "../../utils/storage"

import { getPanel, setPanel } from "../../activate/registerCommands"

Expand Down Expand Up @@ -151,7 +152,7 @@
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"
Expand Down Expand Up @@ -2771,7 +2772,11 @@
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" })
Expand All @@ -2795,6 +2800,183 @@
}
}

public async prepareHistoryItemForResume<T extends HistoryItem>(historyItem: T): Promise<T | undefined> {
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
}

// 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(
`This conversation was created in "${originalWorkspace}", but the current workspace is "${currentWorkspace}". ` +

Check warning on line 2824 in src/core/webview/ClineProvider.ts

View workflow job for this annotation

GitHub Actions / mutation-diff

Mutation test advisory

src/core/webview/ClineProvider.ts:2824: Survived StringLiteral mutant (replacement: ``). See the job summary for the complete list and resolution guidance.
"Choose where to continue. Using the current workspace resets checkpoints created in the original workspace.",

Check warning on line 2825 in src/core/webview/ClineProvider.ts

View workflow job for this annotation

GitHub Actions / mutation-diff

Mutation test advisory

src/core/webview/ClineProvider.ts:2825: Survived StringLiteral mutant (replacement: ""). See the job summary for the complete list and resolution guidance.
{ 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
}
Comment thread
PierrunoYT marked this conversation as resolved.
Resolved

// 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`)

Check warning on line 2844 in src/core/webview/ClineProvider.ts

View workflow job for this annotation

GitHub Actions / mutation-diff

Mutation test advisory

src/core/webview/ClineProvider.ts:2844: Survived StringLiteral mutant (replacement: ``). See the job summary for the complete list and resolution guidance.
return undefined
}

const updatedHistoryItem = { ...historyItem, workspace: currentWorkspace }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

historyItem is read before the modal, and the modal has no timeout. Should this re-read taskHistoryStore.get(id) after the prompt and build the update from {...fresh, workspace}? The store writes a delta against its cache, so a stale status or pendingAction could overwrite newer state, and the rollback at line 2401 has the same stale base.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, fixed in c47b767. Both the forward write and the rollback now go through atomicReadAndUpdate on the current stored record and change only workspace, so a status/pendingAction written while the modal is open is never overwritten from the pre-prompt snapshot. The rollback only reverts when the stored workspace is still the one we set, so a newer move is left alone. prepareHistoryItemForResume now returns the record as persisted ({...historyItem, ...persisted, workspace}) rather than the stale snapshot.

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<HistoryItem | undefined> {
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

Check warning on line 2879 in src/core/webview/ClineProvider.ts

View workflow job for this annotation

GitHub Actions / mutation-diff

Mutation test advisory

src/core/webview/ClineProvider.ts:2879: Survived ConditionalExpression mutant (replacement: true). See the job summary for the complete list and resolution guidance.
return { ...current, workspace }
})
Comment thread
PierrunoYT marked this conversation as resolved.
Resolved
this.recentTasksCache = undefined
const updated = this.taskHistoryStore.get(taskId)
if (changed && updated && this.isViewLaunched) {
await this.postMessageToWebview({ type: "taskHistoryItemUpdated", taskHistoryItem: updated })

Check warning on line 2885 in src/core/webview/ClineProvider.ts

View workflow job for this annotation

GitHub Actions / mutation-diff

Mutation test advisory

src/core/webview/ClineProvider.ts:2885: 2 mutation test gaps; example: Survived ObjectLiteral mutant (replacement: {}). See the job summary for the complete list and resolution guidance.
}
return updated
}

private async resetTaskCheckpointsForWorkspaceChange(
originalHistoryItem: HistoryItem,
updatedHistoryItem: HistoryItem,
): Promise<HistoryItem | undefined> {
const taskId = originalHistoryItem.id
const globalStoragePath = this.contextProxy.globalStorageUri.fsPath
let removedCheckpoints: ClineMessage[] = []

Check warning on line 2896 in src/core/webview/ClineProvider.ts

View workflow job for this annotation

GitHub Actions / mutation-diff

Mutation test advisory

src/core/webview/ClineProvider.ts:2896: Survived ArrayDeclaration mutant (replacement: ["Stryker was here"]). See the job summary for the complete list and resolution guidance.
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()}`)
let checkpointDirectoryStaged = false

Check warning on line 2901 in src/core/webview/ClineProvider.ts

View workflow job for this annotation

GitHub Actions / mutation-diff

Mutation test advisory

src/core/webview/ClineProvider.ts:2901: Survived BooleanLiteral mutant (replacement: true). See the job summary for the complete list and resolution guidance.

try {
await fs.rename(checkpointsDir, checkpointBackupDir)
checkpointDirectoryStaged = true
} catch (error) {
if (!(error instanceof Error && "code" in error && error.code === "ENOENT")) {
throw error
}
}

try {
// 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)
Comment thread
PierrunoYT marked this conversation as resolved.
Resolved
} catch (error) {
if (removedCheckpoints.length > 0) {
try {
// 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)}`,
)
}
}
if (checkpointDirectoryStaged) {
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)}`,
)
}
}
try {
// 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)}`,
)
}
throw error
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
}

if (checkpointDirectoryStaged) {
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)
}
}
}

return persisted
}

async exportTaskWithId(id: string) {
const { historyItem, apiConversationHistory } = await this.getTaskWithId(id)
const fileName = getTaskFileName(historyItem.ts)
Expand Down
Loading
Loading