diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 86ce5d8e67..378487ffe5 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -243,8 +243,6 @@ export class ClineProvider private recentTasksCache?: string[] public readonly taskHistoryStore: TaskHistoryStore private taskHistoryStoreInitialized = false - private globalStateWriteThroughTimer: ReturnType | null = null - private static readonly GLOBAL_STATE_WRITE_THROUGH_DEBOUNCE_MS = 5000 // 5 seconds public static readonly PENDING_OPERATION_TIMEOUT_MS = 30000 // 30 seconds private providerProfileMutationQueue = Promise.resolve() private historyTaskCreationQueue = Promise.resolve() @@ -344,14 +342,8 @@ export class ClineProvider this.mdmService = mdmService void this.updateGlobalState("codebaseIndexModels", EMBEDDING_MODEL_PROFILES) - // Initialize the per-task file-based history store. - // The globalState write-through is debounced separately (not on every mutation) - // since per-task files are authoritative and globalState is only for downgrade compat. - this.taskHistoryStore = new TaskHistoryStore(this.contextProxy.globalStorageUri.fsPath, { - onWrite: async () => { - this.scheduleGlobalStateWriteThrough() - }, - }) + // Initialize the authoritative per-task file-based history store. + this.taskHistoryStore = new TaskHistoryStore(this.contextProxy.globalStorageUri.fsPath) this.initializeTaskHistoryStore().catch((error) => { this.log(`Failed to initialize TaskHistoryStore: ${error}`) }) @@ -897,7 +889,6 @@ export class ClineProvider await this.marketplaceManager?.cleanup() this.customModesManager?.dispose() this.taskHistoryStore.dispose() - this.flushGlobalStateWriteThrough() this.log("Disposed all disposables") ClineProvider.activeInstances.delete(this) @@ -1743,9 +1734,7 @@ export class ClineProvider try { // Update the task history with the new mode first. - const taskHistoryItem = - this.taskHistoryStore.get(task.taskId) ?? - (this.getGlobalState("taskHistory") ?? []).find((item) => item.id === task.taskId) + const taskHistoryItem = this.getTaskHistoryItem(task.taskId) if (taskHistoryItem) { await this.updateTaskHistory({ ...taskHistoryItem, mode: newMode }) @@ -1983,9 +1972,7 @@ export class ClineProvider // been persisted into taskHistory (it will be captured on the next save). task.setTaskApiConfigName(apiConfigName) - const taskHistoryItem = - this.taskHistoryStore.get(task.taskId) ?? - (this.getGlobalState("taskHistory") ?? []).find((item) => item.id === task.taskId) + const taskHistoryItem = this.getTaskHistoryItem(task.taskId) if (taskHistoryItem) { await this.updateTaskHistory({ ...taskHistoryItem, apiConfigName }) @@ -2241,6 +2228,18 @@ export class ClineProvider // Task history + private getTaskHistoryItem(id: string): HistoryItem | undefined { + const historyItem = this.taskHistoryStore.get(id) + + // Once initialization and migration succeed, the file-backed store is authoritative. + // Legacy global state is only a fallback while startup is incomplete or has failed. + if (historyItem || this.taskHistoryStoreInitialized) { + return historyItem + } + + return (this.getGlobalState("taskHistory") ?? []).find((item) => item.id === id) + } + async getTaskWithId(id: string): Promise<{ historyItem: HistoryItem taskDirPath: string @@ -2248,8 +2247,7 @@ export class ClineProvider uiMessagesFilePath: string apiConversationHistory: Anthropic.MessageParam[] }> { - const historyItem = - this.taskHistoryStore.get(id) ?? (this.getGlobalState("taskHistory") ?? []).find((item) => item.id === id) + const historyItem = this.getTaskHistoryItem(id) if (!historyItem) { throw new Error("Task not found") @@ -3134,44 +3132,6 @@ export class ClineProvider return history } - /** - * Schedule a debounced write-through of task history to globalState. - * Only used for backward compatibility during the transition period. - * Per-task files are authoritative; globalState is the downgrade fallback. - */ - private scheduleGlobalStateWriteThrough(): void { - if (this.globalStateWriteThroughTimer) { - clearTimeout(this.globalStateWriteThroughTimer) - } - - this.globalStateWriteThroughTimer = setTimeout(async () => { - this.globalStateWriteThroughTimer = null - try { - const items = this.taskHistoryStore.getAll() - await this.updateGlobalState("taskHistory", items) - } catch (err) { - this.log( - `[scheduleGlobalStateWriteThrough] Failed: ${err instanceof Error ? err.message : String(err)}`, - ) - } - }, ClineProvider.GLOBAL_STATE_WRITE_THROUGH_DEBOUNCE_MS) - } - - /** - * Flush any pending debounced globalState write-through immediately. - */ - private flushGlobalStateWriteThrough(): void { - if (this.globalStateWriteThroughTimer) { - clearTimeout(this.globalStateWriteThroughTimer) - this.globalStateWriteThroughTimer = null - } - - const items = this.taskHistoryStore.getAll() - this.updateGlobalState("taskHistory", items).catch((err) => { - this.log(`[flushGlobalStateWriteThrough] Failed: ${err instanceof Error ? err.message : String(err)}`) - }) - } - /** * Broadcasts a task history update to the webview. * This sends a lightweight message with just the task history, rather than the full state. diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts index 97c4dd877e..72aa1bcb76 100644 --- a/src/core/webview/__tests__/ClineProvider.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.spec.ts @@ -4962,6 +4962,36 @@ describe("ClineProvider - Comprehensive Edit/Delete Edge Cases", () => { }) describe("getTaskWithId", () => { + it("does not restore a deleted file-backed task from legacy history", async () => { + const historyItem = { + id: "deleted-task", + task: "legacy task", + ts: Date.now(), + number: 1, + tokensIn: 0, + tokensOut: 0, + totalCost: 0, + } + vi.mocked(mockContext.globalState.get).mockImplementation((key: string) => { + if (key === "taskHistory") { + return [historyItem] + } + return undefined + }) + + provider.taskHistoryStore["cache"].set(historyItem.id, historyItem) + await provider.taskHistoryStore.delete(historyItem.id) + provider["taskHistoryStoreInitialized"] = true + + await expect(provider.getTaskWithId(historyItem.id)).rejects.toThrow("Task not found") + }) + + it("rejects a missing task before file-backed history initialization", async () => { + provider["taskHistoryStoreInitialized"] = false + vi.mocked(mockContext.globalState.get).mockReturnValue(undefined) + await expect(provider.getTaskWithId("cold-start-missing-task")).rejects.toThrow("Task not found") + }) + it("returns empty apiConversationHistory when file is missing", async () => { const historyItem = { id: "missing-api-file-task", task: "test task", ts: Date.now() } vi.mocked(mockContext.globalState.get).mockImplementation((key: string) => { diff --git a/src/core/webview/__tests__/ClineProvider.sticky-mode.spec.ts b/src/core/webview/__tests__/ClineProvider.sticky-mode.spec.ts index fedfa13030..55e26a9667 100644 --- a/src/core/webview/__tests__/ClineProvider.sticky-mode.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.sticky-mode.spec.ts @@ -212,6 +212,12 @@ describe("ClineProvider - Sticky Mode", () => { let mockWebviewView: vscode.WebviewView let mockPostMessage: any + async function seedTaskHistory(items: HistoryItem[]) { + for (const item of items) { + await provider.taskHistoryStore.upsert(item) + } + } + beforeEach(async () => { vi.clearAllMocks() @@ -325,8 +331,8 @@ describe("ClineProvider - Sticky Mode", () => { // Get the actual taskId from the mock const taskId = (mockTask as any).taskId || "test-task-id" - // Mock getGlobalState to return task history - vi.spyOn(provider as any, "getGlobalState").mockReturnValue([ + // Seed the authoritative file-backed task history. + await seedTaskHistory([ { id: taskId, ts: Date.now(), @@ -378,8 +384,8 @@ describe("ClineProvider - Sticky Mode", () => { // Add task to provider stack await provider.addClineToStack(mockTask as any) - // Mock getGlobalState to return task history - vi.spyOn(provider as any, "getGlobalState").mockReturnValue([ + // Seed the authoritative file-backed task history. + await seedTaskHistory([ { id: mockTask.taskId, ts: Date.now(), @@ -418,8 +424,8 @@ describe("ClineProvider - Sticky Mode", () => { // Get the actual taskId from the mock const taskId = (mockTask as any).taskId || "test-task-id" - // Mock getGlobalState to return task history - vi.spyOn(provider as any, "getGlobalState").mockReturnValue([ + // Seed the authoritative file-backed task history. + await seedTaskHistory([ { id: taskId, ts: Date.now(), @@ -541,8 +547,8 @@ describe("ClineProvider - Sticky Mode", () => { // Get the actual taskId from the mock const taskId = (mockTask as any).taskId || "test-task-id" - // Mock getGlobalState to return task history with our task - vi.spyOn(provider as any, "getGlobalState").mockReturnValue([ + // Seed the authoritative file-backed task history. + await seedTaskHistory([ { id: taskId, ts: Date.now(), @@ -599,25 +605,21 @@ describe("ClineProvider - Sticky Mode", () => { [parentTaskId]: "architect", // Parent starts with architect mode } - // Mock getGlobalState to return task history - const getGlobalStateMock = vi.spyOn(provider as any, "getGlobalState") - getGlobalStateMock.mockImplementation((key) => { - if (key === "taskHistory") { - return Object.entries(taskModes).map(([id, mode]) => ({ - id, - ts: Date.now(), - task: `Task ${id}`, - number: 1, - tokensIn: 0, - tokensOut: 0, - cacheWrites: 0, - cacheReads: 0, - totalCost: 0, - mode, - })) - } - // Return empty array for other keys - return [] + // Read task metadata from the authoritative store's test double. + vi.spyOn(provider.taskHistoryStore, "get").mockImplementation((id) => { + const mode = taskModes[id] + return mode === undefined + ? undefined + : { + id, + ts: Date.now(), + task: `Task ${id}`, + number: 1, + tokensIn: 0, + tokensOut: 0, + totalCost: 0, + mode, + } }) // Mock updateTaskHistory to track mode changes @@ -828,8 +830,8 @@ describe("ClineProvider - Sticky Mode", () => { // Add task to provider stack await provider.addClineToStack(mockTask as any) - // Mock getGlobalState to return task history - vi.spyOn(provider as any, "getGlobalState").mockReturnValue([ + // Seed the authoritative file-backed task history. + await seedTaskHistory([ { id: mockTask.taskId, ts: Date.now(), @@ -895,8 +897,8 @@ describe("ClineProvider - Sticky Mode", () => { // Add task to provider stack await provider.addClineToStack(mockTask as any) - // Mock getGlobalState - vi.spyOn(provider as any, "getGlobalState").mockReturnValue([ + // Seed the authoritative file-backed task history. + await seedTaskHistory([ { id: mockTask.taskId, ts: Date.now(), @@ -984,8 +986,8 @@ describe("ClineProvider - Sticky Mode", () => { // Add task to provider stack await provider.addClineToStack(mockTask as any) - // Mock getGlobalState to return task history - vi.spyOn(provider as any, "getGlobalState").mockReturnValue([ + // Seed the authoritative file-backed task history. + await seedTaskHistory([ { id: mockTask.taskId, ts: Date.now(), @@ -1042,8 +1044,8 @@ describe("ClineProvider - Sticky Mode", () => { // Add task to provider stack await provider.addClineToStack(mockTask as any) - // Mock getGlobalState - vi.spyOn(provider as any, "getGlobalState").mockReturnValue([ + // Seed the authoritative file-backed task history. + await seedTaskHistory([ { id: mockTask.taskId, ts: Date.now(), @@ -1111,8 +1113,8 @@ describe("ClineProvider - Sticky Mode", () => { await provider.addClineToStack(task2 as any) await provider.addClineToStack(task3 as any) - // Mock getGlobalState to return all tasks - vi.spyOn(provider as any, "getGlobalState").mockReturnValue([ + // Seed the authoritative file-backed task history. + await seedTaskHistory([ { id: task1.taskId, ts: Date.now(), diff --git a/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts b/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts index 2bbf0736c6..61254a67d6 100644 --- a/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts @@ -243,6 +243,7 @@ vi.mock("@roo-code/cloud", () => ({ getOrganizationMemberships: vi.fn().mockResolvedValue([]), getUserSettings: vi.fn().mockReturnValue(null), isTaskSyncEnabled: vi.fn().mockReturnValue(false), + off: vi.fn(), } }, }, @@ -386,6 +387,32 @@ describe("ClineProvider Task History Synchronization", () => { return calls.filter((call) => call[0]?.type === type) } + it("uses per-task files without registering a globalState write-through callback", () => { + expect(provider.taskHistoryStore["onWrite"]).toBeUndefined() + }) + + it("does not write task history to globalState after a history mutation", async () => { + vi.mocked(mockContext.globalState.update).mockClear() + + await provider.updateTaskHistory(createHistoryItem({ id: "file-backed-task", task: "File-backed task" }), { + broadcast: false, + }) + + expect(mockContext.globalState.update).not.toHaveBeenCalledWith("taskHistory", expect.anything()) + }) + + it("does not write task history to globalState during disposal", async () => { + await provider.updateTaskHistory( + createHistoryItem({ id: "disposed-file-backed-task", task: "Disposed file-backed task" }), + { broadcast: false }, + ) + vi.mocked(mockContext.globalState.update).mockClear() + + await provider.dispose() + + expect(mockContext.globalState.update).not.toHaveBeenCalledWith("taskHistory", expect.anything()) + }) + describe("updateTaskHistory", () => { it("broadcasts task history update by default", async () => { await provider.resolveWebviewView(mockWebviewView) diff --git a/src/eslint-suppressions.json b/src/eslint-suppressions.json index 93741e9174..83b08c98a3 100644 --- a/src/eslint-suppressions.json +++ b/src/eslint-suppressions.json @@ -1041,7 +1041,7 @@ }, "core/webview/__tests__/ClineProvider.sticky-mode.spec.ts": { "@typescript-eslint/no-explicit-any": { - "count": 37 + "count": 27 } }, "core/webview/__tests__/ClineProvider.sticky-profile.spec.ts": {