From ab3070884d8d03e8e0cdee2ba8bcc1a9c8ae10ab Mon Sep 17 00:00:00 2001 From: Roo Code Date: Mon, 16 Feb 2026 18:10:33 +0000 Subject: [PATCH 1/4] feat: add per-task file-based history store for cross-instance safety Implement TaskHistoryStore service that stores each task's HistoryItem as an individual JSON file in its existing task directory. This prevents silent data loss when multiple VS Code windows write to the shared globalState taskHistory array concurrently. Key changes: - New TaskHistoryStore class with per-task file writes via safeWriteJson - Index file (_index.json) for fast startup reads - Reconciliation logic to detect and fix drift between instances - fs.watch for cross-instance reactivity - Debounced index writes (2s window) for streaming performance - Migration from globalState on first startup - Write-through to globalState during transition period - Fallback lookups from globalState for backward compatibility Files created: - src/core/task-persistence/TaskHistoryStore.ts - src/core/task-persistence/__tests__/TaskHistoryStore.spec.ts - src/core/task-persistence/__tests__/TaskHistoryStore.crossInstance.spec.ts Files modified: - src/shared/globalFileNames.ts (added historyItem, historyIndex) - src/core/task-persistence/index.ts (export TaskHistoryStore) - src/core/webview/ClineProvider.ts (integrate store, remove write lock) - Test files updated for new store-based approach --- src/core/task-persistence/TaskHistoryStore.ts | 523 ++++++++++++++++++ .../TaskHistoryStore.crossInstance.spec.ts | 165 ++++++ .../__tests__/TaskHistoryStore.spec.ts | 442 +++++++++++++++ src/core/task-persistence/index.ts | 1 + src/core/webview/ClineProvider.ts | 152 ++--- .../ClineProvider.sticky-mode.spec.ts | 18 +- .../ClineProvider.sticky-profile.spec.ts | 95 ++-- .../ClineProvider.taskHistory.spec.ts | 55 +- src/shared/globalFileNames.ts | 2 + 9 files changed, 1316 insertions(+), 137 deletions(-) create mode 100644 src/core/task-persistence/TaskHistoryStore.ts create mode 100644 src/core/task-persistence/__tests__/TaskHistoryStore.crossInstance.spec.ts create mode 100644 src/core/task-persistence/__tests__/TaskHistoryStore.spec.ts diff --git a/src/core/task-persistence/TaskHistoryStore.ts b/src/core/task-persistence/TaskHistoryStore.ts new file mode 100644 index 00000000000..e542d27853d --- /dev/null +++ b/src/core/task-persistence/TaskHistoryStore.ts @@ -0,0 +1,523 @@ +import * as fs from "fs/promises" +import * as fsSync from "fs" +import * as path from "path" + +import type { HistoryItem } from "@roo-code/types" + +import { GlobalFileNames } from "../../shared/globalFileNames" +import { safeWriteJson } from "../../utils/safeWriteJson" +import { getStorageBasePath } from "../../utils/storage" + +/** + * Index file format for fast startup reads. + */ +interface HistoryIndex { + version: number + updatedAt: number + entries: HistoryItem[] +} + +/** + * TaskHistoryStore encapsulates all task history persistence logic. + * + * Each task's HistoryItem is stored as an individual JSON file in its + * existing task directory (`globalStorage/tasks//history_item.json`). + * A single index file (`globalStorage/tasks/_index.json`) is maintained + * as a cache for fast list reads at startup. + * + * Cross-process safety comes from `safeWriteJson`'s `proper-lockfile` + * on per-task file writes. Within a single extension host process, + * an in-process write lock serializes mutations. + */ +export class TaskHistoryStore { + private readonly globalStoragePath: string + private cache: Map = new Map() + private writeLock: Promise = Promise.resolve() + private indexWriteTimer: ReturnType | null = null + private fsWatcher: fsSync.FSWatcher | null = null + private reconcileTimer: ReturnType | null = null + private disposed = false + + /** Debounce window for index writes in milliseconds. */ + private static readonly INDEX_WRITE_DEBOUNCE_MS = 2000 + + /** Periodic reconciliation interval in milliseconds. */ + private static readonly RECONCILE_INTERVAL_MS = 5 * 60 * 1000 + + constructor(globalStoragePath: string) { + this.globalStoragePath = globalStoragePath + } + + // ────────────────────────────── Lifecycle ────────────────────────────── + + /** + * Load index, reconcile if needed, start watchers. + */ + async initialize(): Promise { + const tasksDir = await this.getTasksDir() + await fs.mkdir(tasksDir, { recursive: true }) + + // 1. Load existing index into the cache + await this.loadIndex() + + // 2. Reconcile cache against actual task directories on disk + await this.reconcile() + + // 3. Start fs.watch for cross-instance reactivity + this.startWatcher() + + // 4. Start periodic reconciliation as a defensive fallback + this.startPeriodicReconciliation() + } + + /** + * Flush pending writes, clear watchers, release resources. + */ + dispose(): void { + this.disposed = true + + if (this.indexWriteTimer) { + clearTimeout(this.indexWriteTimer) + this.indexWriteTimer = null + } + + if (this.reconcileTimer) { + clearTimeout(this.reconcileTimer) + this.reconcileTimer = null + } + + if (this.fsWatcher) { + this.fsWatcher.close() + this.fsWatcher = null + } + + // Synchronously flush the index (best-effort) + this.flushIndex().catch((err) => { + console.error("[TaskHistoryStore] Error flushing index on dispose:", err) + }) + } + + // ────────────────────────────── Reads ────────────────────────────── + + /** + * Get a single history item by task ID. + */ + get(taskId: string): HistoryItem | undefined { + return this.cache.get(taskId) + } + + /** + * Get all history items, sorted by timestamp descending (newest first). + */ + getAll(): HistoryItem[] { + return Array.from(this.cache.values()).sort((a, b) => b.ts - a.ts) + } + + /** + * Get history items filtered by workspace path. + */ + getByWorkspace(workspace: string): HistoryItem[] { + return this.getAll().filter((item) => item.workspace === workspace) + } + + // ────────────────────────────── Mutations ────────────────────────────── + + /** + * Insert or update a history item. + * + * Writes the per-task file immediately (source of truth), + * updates the in-memory Map, and schedules a debounced index write. + */ + async upsert(item: HistoryItem): Promise { + return this.withLock(async () => { + const existing = this.cache.get(item.id) + + // Merge: preserve existing metadata unless explicitly overwritten + const merged = existing ? { ...existing, ...item } : item + + // Write per-task file (source of truth) + await this.writeTaskFile(merged) + + // Update in-memory cache + this.cache.set(merged.id, merged) + + // Schedule debounced index write + this.scheduleIndexWrite() + + return this.getAll() + }) + } + + /** + * Delete a single task's history item. + */ + async delete(taskId: string): Promise { + return this.withLock(async () => { + this.cache.delete(taskId) + + // Remove per-task file (best-effort) + try { + const filePath = await this.getTaskFilePath(taskId) + await fs.unlink(filePath) + } catch { + // File may already be deleted + } + + this.scheduleIndexWrite() + }) + } + + /** + * Delete multiple tasks' history items in a batch. + */ + async deleteMany(taskIds: string[]): Promise { + return this.withLock(async () => { + for (const taskId of taskIds) { + this.cache.delete(taskId) + + try { + const filePath = await this.getTaskFilePath(taskId) + await fs.unlink(filePath) + } catch { + // File may already be deleted + } + } + + this.scheduleIndexWrite() + }) + } + + // ────────────────────────────── Reconciliation ────────────────────────────── + + /** + * Scan task directories vs index and fix any drift. + * + * - Tasks on disk but missing from cache: read and add + * - Tasks in cache but missing from disk: remove + */ + async reconcile(): Promise { + const tasksDir = await this.getTasksDir() + + let dirEntries: string[] + try { + dirEntries = await fs.readdir(tasksDir) + } catch { + return // tasks dir doesn't exist yet + } + + // Filter out the index file and hidden files + const taskDirNames = dirEntries.filter((name) => !name.startsWith("_") && !name.startsWith(".")) + + const onDiskIds = new Set(taskDirNames) + const cacheIds = new Set(this.cache.keys()) + let changed = false + + // Tasks on disk but not in cache: read their history_item.json + for (const taskId of onDiskIds) { + if (!cacheIds.has(taskId)) { + try { + const item = await this.readTaskFile(taskId) + if (item) { + this.cache.set(taskId, item) + changed = true + } + } catch { + // Corrupted or missing file, skip + } + } + } + + // Tasks in cache but not on disk: remove from cache + for (const taskId of cacheIds) { + if (!onDiskIds.has(taskId)) { + this.cache.delete(taskId) + changed = true + } + } + + if (changed) { + this.scheduleIndexWrite() + } + } + + // ────────────────────────────── Cache invalidation ────────────────────────────── + + /** + * Invalidate a single task's cache entry (re-read from disk on next access). + */ + async invalidate(taskId: string): Promise { + try { + const item = await this.readTaskFile(taskId) + if (item) { + this.cache.set(taskId, item) + } else { + this.cache.delete(taskId) + } + } catch { + this.cache.delete(taskId) + } + } + + /** + * Clear all in-memory cache and reload from index. + */ + invalidateAll(): void { + this.cache.clear() + } + + // ────────────────────────────── Migration ────────────────────────────── + + /** + * Migrate from globalState taskHistory array to per-task files. + * + * For each entry in the globalState array, writes a `history_item.json` + * file if one doesn't already exist. This is idempotent and safe to re-run. + */ + async migrateFromGlobalState(taskHistoryEntries: HistoryItem[]): Promise { + if (!taskHistoryEntries || taskHistoryEntries.length === 0) { + return + } + + for (const item of taskHistoryEntries) { + if (!item.id) { + continue + } + + // Check if task directory exists on disk + const tasksDir = await this.getTasksDir() + const taskDir = path.join(tasksDir, item.id) + + try { + await fs.access(taskDir) + } catch { + // Task directory doesn't exist; skip this entry as it's orphaned in globalState + continue + } + + // Write history_item.json if it doesn't exist yet + const filePath = path.join(taskDir, GlobalFileNames.historyItem) + try { + await fs.access(filePath) + // File already exists, skip (don't overwrite existing per-task files) + } catch { + // File doesn't exist, write it + await safeWriteJson(filePath, item) + this.cache.set(item.id, item) + } + } + + // Write the index + await this.writeIndex() + } + + // ────────────────────────────── Private: Index management ────────────────────────────── + + /** + * Load the `_index.json` file into the in-memory cache. + */ + private async loadIndex(): Promise { + const indexPath = await this.getIndexPath() + + try { + const raw = await fs.readFile(indexPath, "utf8") + const index: HistoryIndex = JSON.parse(raw) + + if (index.version === 1 && Array.isArray(index.entries)) { + for (const entry of index.entries) { + if (entry.id) { + this.cache.set(entry.id, entry) + } + } + } + } catch { + // Index doesn't exist or is corrupted; cache stays empty. + // Reconciliation will rebuild it from per-task files. + } + } + + /** + * Write the full index to disk. + */ + private async writeIndex(): Promise { + const indexPath = await this.getIndexPath() + const index: HistoryIndex = { + version: 1, + updatedAt: Date.now(), + entries: this.getAll(), + } + + await safeWriteJson(indexPath, index) + } + + /** + * Schedule a debounced index write. + */ + private scheduleIndexWrite(): void { + if (this.disposed) { + return + } + + if (this.indexWriteTimer) { + clearTimeout(this.indexWriteTimer) + } + + this.indexWriteTimer = setTimeout(async () => { + this.indexWriteTimer = null + try { + await this.writeIndex() + } catch (err) { + console.error("[TaskHistoryStore] Failed to write index:", err) + } + }, TaskHistoryStore.INDEX_WRITE_DEBOUNCE_MS) + } + + /** + * Force an immediate index write (called on dispose/shutdown). + */ + async flushIndex(): Promise { + if (this.indexWriteTimer) { + clearTimeout(this.indexWriteTimer) + this.indexWriteTimer = null + } + + await this.writeIndex() + } + + // ────────────────────────────── Private: Per-task file I/O ────────────────────────────── + + /** + * Write a HistoryItem to its per-task `history_item.json` file. + */ + private async writeTaskFile(item: HistoryItem): Promise { + const filePath = await this.getTaskFilePath(item.id) + await safeWriteJson(filePath, item) + } + + /** + * Read a HistoryItem from its per-task `history_item.json` file. + */ + private async readTaskFile(taskId: string): Promise { + const filePath = await this.getTaskFilePath(taskId) + + try { + const raw = await fs.readFile(filePath, "utf8") + const item: HistoryItem = JSON.parse(raw) + return item.id ? item : null + } catch { + return null + } + } + + // ────────────────────────────── Private: fs.watch ────────────────────────────── + + /** + * Watch the tasks directory for changes from other instances. + */ + private startWatcher(): void { + if (this.disposed) { + return + } + + // Use a debounced handler to avoid excessive reconciliation + let watchDebounce: ReturnType | null = null + + this.getTasksDir() + .then((tasksDir) => { + if (this.disposed) { + return + } + + try { + this.fsWatcher = fsSync.watch(tasksDir, { recursive: false }, (_eventType, _filename) => { + if (this.disposed) { + return + } + + // Debounce the reconciliation triggered by fs.watch + if (watchDebounce) { + clearTimeout(watchDebounce) + } + watchDebounce = setTimeout(() => { + this.reconcile().catch((err) => { + console.error("[TaskHistoryStore] Reconciliation after fs.watch failed:", err) + }) + }, 500) + }) + + this.fsWatcher.on("error", (err) => { + console.error("[TaskHistoryStore] fs.watch error:", err) + // fs.watch is unreliable on some platforms; periodic reconciliation + // serves as the fallback. + }) + } catch (err) { + console.error("[TaskHistoryStore] Failed to start fs.watch:", err) + } + }) + .catch((err) => { + console.error("[TaskHistoryStore] Failed to get tasks dir for watcher:", err) + }) + } + + /** + * Start periodic reconciliation as a defensive fallback for platforms + * where fs.watch is unreliable. + */ + private startPeriodicReconciliation(): void { + if (this.disposed) { + return + } + + this.reconcileTimer = setTimeout(async () => { + if (this.disposed) { + return + } + try { + await this.reconcile() + } catch (err) { + console.error("[TaskHistoryStore] Periodic reconciliation failed:", err) + } + this.startPeriodicReconciliation() + }, TaskHistoryStore.RECONCILE_INTERVAL_MS) + } + + // ────────────────────────────── Private: Write lock ────────────────────────────── + + /** + * Serializes all read-modify-write operations within a single extension + * host process to prevent concurrent interleaving. + */ + private withLock(fn: () => Promise): Promise { + const result = this.writeLock.then(fn, fn) + this.writeLock = result.then( + () => {}, + () => {}, + ) + return result + } + + // ────────────────────────────── Private: Path helpers ────────────────────────────── + + /** + * Get the tasks base directory path, resolving custom storage paths. + */ + private async getTasksDir(): Promise { + const basePath = await getStorageBasePath(this.globalStoragePath) + return path.join(basePath, "tasks") + } + + /** + * Get the path to a task's `history_item.json` file. + */ + private async getTaskFilePath(taskId: string): Promise { + const tasksDir = await this.getTasksDir() + return path.join(tasksDir, taskId, GlobalFileNames.historyItem) + } + + /** + * Get the path to the `_index.json` file. + */ + private async getIndexPath(): Promise { + const tasksDir = await this.getTasksDir() + return path.join(tasksDir, GlobalFileNames.historyIndex) + } +} diff --git a/src/core/task-persistence/__tests__/TaskHistoryStore.crossInstance.spec.ts b/src/core/task-persistence/__tests__/TaskHistoryStore.crossInstance.spec.ts new file mode 100644 index 00000000000..f344e58dfd8 --- /dev/null +++ b/src/core/task-persistence/__tests__/TaskHistoryStore.crossInstance.spec.ts @@ -0,0 +1,165 @@ +// pnpm --filter roo-cline test core/task-persistence/__tests__/TaskHistoryStore.crossInstance.spec.ts + +import * as fs from "fs/promises" +import * as path from "path" +import * as os from "os" + +import type { HistoryItem } from "@roo-code/types" + +import { TaskHistoryStore } from "../TaskHistoryStore" +import { GlobalFileNames } from "../../../shared/globalFileNames" + +vi.mock("../../../utils/storage", () => ({ + getStorageBasePath: vi.fn().mockImplementation((defaultPath: string) => defaultPath), +})) + +// Mock safeWriteJson to use plain fs writes in tests (avoids proper-lockfile issues) +vi.mock("../../../utils/safeWriteJson", () => ({ + safeWriteJson: vi.fn().mockImplementation(async (filePath: string, data: any) => { + await fs.mkdir(path.dirname(filePath), { recursive: true }) + await fs.writeFile(filePath, JSON.stringify(data, null, "\t"), "utf8") + }), +})) + +function makeHistoryItem(overrides: Partial = {}): HistoryItem { + return { + id: `task-${Date.now()}-${Math.random().toString(36).substring(2, 8)}`, + number: 1, + ts: Date.now(), + task: "Test task", + tokensIn: 100, + tokensOut: 50, + totalCost: 0.01, + workspace: "/test/workspace", + ...overrides, + } +} + +describe("TaskHistoryStore cross-instance safety", () => { + let tmpDir: string + let storeA: TaskHistoryStore + let storeB: TaskHistoryStore + + beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "task-history-cross-")) + // Two stores pointing at the same globalStoragePath (simulating two VS Code windows) + storeA = new TaskHistoryStore(tmpDir) + storeB = new TaskHistoryStore(tmpDir) + }) + + afterEach(async () => { + storeA.dispose() + storeB.dispose() + await fs.rm(tmpDir, { recursive: true, force: true }).catch(() => {}) + }) + + it("two instances can write different tasks without conflict", async () => { + await storeA.initialize() + await storeB.initialize() + + // Instance A writes task-a + await storeA.upsert(makeHistoryItem({ id: "task-a", task: "Task from instance A" })) + + // Instance B writes task-b + await storeB.upsert(makeHistoryItem({ id: "task-b", task: "Task from instance B" })) + + // Each instance sees its own task + expect(storeA.get("task-a")).toBeDefined() + expect(storeB.get("task-b")).toBeDefined() + + // After reconciliation, instance A should see task-b and vice versa + await storeA.reconcile() + await storeB.reconcile() + + expect(storeA.get("task-b")).toBeDefined() + expect(storeB.get("task-a")).toBeDefined() + + expect(storeA.getAll()).toHaveLength(2) + expect(storeB.getAll()).toHaveLength(2) + }) + + it("reconciliation in instance B detects a task created by instance A", async () => { + await storeA.initialize() + await storeB.initialize() + + // Instance A creates a task + const item = makeHistoryItem({ id: "cross-task", task: "Created by A" }) + await storeA.upsert(item) + + // Instance B doesn't know about it yet + expect(storeB.get("cross-task")).toBeUndefined() + + // Reconciliation picks it up + await storeB.reconcile() + + expect(storeB.get("cross-task")).toBeDefined() + expect(storeB.get("cross-task")!.task).toBe("Created by A") + }) + + it("delete by instance A is detected by instance B reconciliation", async () => { + await storeA.initialize() + await storeB.initialize() + + // Both instances have a task + const item = makeHistoryItem({ id: "shared-task" }) + await storeA.upsert(item) + await storeB.reconcile() // B picks it up + + expect(storeB.get("shared-task")).toBeDefined() + + // Instance A deletes the task (per-task file + directory would be removed) + await storeA.delete("shared-task") + + // Remove the task directory to simulate full deletion (deleteTaskWithId removes the dir) + const taskDir = path.join(tmpDir, "tasks", "shared-task") + await fs.rm(taskDir, { recursive: true, force: true }) + + // Instance B still has it in cache + expect(storeB.get("shared-task")).toBeDefined() + + // After reconciliation, instance B sees it's gone + await storeB.reconcile() + expect(storeB.get("shared-task")).toBeUndefined() + }) + + it("per-task file updates by one instance are visible to another after invalidation", async () => { + await storeA.initialize() + await storeB.initialize() + + // Instance A creates a task + const item = makeHistoryItem({ id: "update-task", tokensIn: 100 }) + await storeA.upsert(item) + + // Instance B picks it up via reconciliation + await storeB.reconcile() + expect(storeB.get("update-task")!.tokensIn).toBe(100) + + // Instance A updates the task + await storeA.upsert({ ...item, tokensIn: 500 }) + + // Instance B invalidates and re-reads + await storeB.invalidate("update-task") + expect(storeB.get("update-task")!.tokensIn).toBe(500) + }) + + it("concurrent writes to different tasks from two instances produce correct final state", async () => { + await storeA.initialize() + await storeB.initialize() + + // Write alternating tasks from each instance + const promises = [] + for (let i = 0; i < 5; i++) { + promises.push(storeA.upsert(makeHistoryItem({ id: `a-task-${i}`, ts: 1000 + i }))) + promises.push(storeB.upsert(makeHistoryItem({ id: `b-task-${i}`, ts: 2000 + i }))) + } + + await Promise.all(promises) + + // After reconciliation, both should see all 10 tasks + await storeA.reconcile() + await storeB.reconcile() + + expect(storeA.getAll().length).toBe(10) + expect(storeB.getAll().length).toBe(10) + }) +}) diff --git a/src/core/task-persistence/__tests__/TaskHistoryStore.spec.ts b/src/core/task-persistence/__tests__/TaskHistoryStore.spec.ts new file mode 100644 index 00000000000..8adc486160a --- /dev/null +++ b/src/core/task-persistence/__tests__/TaskHistoryStore.spec.ts @@ -0,0 +1,442 @@ +// pnpm --filter roo-cline test core/task-persistence/__tests__/TaskHistoryStore.spec.ts + +import * as fs from "fs/promises" +import * as path from "path" +import * as os from "os" + +import type { HistoryItem } from "@roo-code/types" + +import { TaskHistoryStore } from "../TaskHistoryStore" +import { GlobalFileNames } from "../../../shared/globalFileNames" + +vi.mock("../../../utils/storage", () => ({ + getStorageBasePath: vi.fn().mockImplementation((defaultPath: string) => defaultPath), +})) + +// Mock safeWriteJson to use plain fs writes in tests (avoids proper-lockfile issues) +vi.mock("../../../utils/safeWriteJson", () => ({ + safeWriteJson: vi.fn().mockImplementation(async (filePath: string, data: any) => { + await fs.mkdir(path.dirname(filePath), { recursive: true }) + await fs.writeFile(filePath, JSON.stringify(data, null, "\t"), "utf8") + }), +})) + +function makeHistoryItem(overrides: Partial = {}): HistoryItem { + return { + id: `task-${Date.now()}-${Math.random().toString(36).substring(2, 8)}`, + number: 1, + ts: Date.now(), + task: "Test task", + tokensIn: 100, + tokensOut: 50, + totalCost: 0.01, + workspace: "/test/workspace", + ...overrides, + } +} + +describe("TaskHistoryStore", () => { + let tmpDir: string + let store: TaskHistoryStore + + beforeEach(async () => { + tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "task-history-test-")) + store = new TaskHistoryStore(tmpDir) + }) + + afterEach(async () => { + store.dispose() + await fs.rm(tmpDir, { recursive: true, force: true }).catch(() => {}) + }) + + describe("initialize()", () => { + it("initializes from empty state (no index, no task dirs)", async () => { + await store.initialize() + expect(store.getAll()).toEqual([]) + }) + + it("initializes from existing index file", async () => { + const tasksDir = path.join(tmpDir, "tasks") + await fs.mkdir(tasksDir, { recursive: true }) + + const item1 = makeHistoryItem({ id: "task-1", ts: 1000 }) + const item2 = makeHistoryItem({ id: "task-2", ts: 2000 }) + + // Create task directories so reconciliation doesn't remove them + await fs.mkdir(path.join(tasksDir, "task-1"), { recursive: true }) + await fs.mkdir(path.join(tasksDir, "task-2"), { recursive: true }) + + // Write per-task files + await fs.writeFile(path.join(tasksDir, "task-1", GlobalFileNames.historyItem), JSON.stringify(item1)) + await fs.writeFile(path.join(tasksDir, "task-2", GlobalFileNames.historyItem), JSON.stringify(item2)) + + // Write index + const index = { + version: 1, + updatedAt: Date.now(), + entries: [item1, item2], + } + await fs.writeFile(path.join(tasksDir, GlobalFileNames.historyIndex), JSON.stringify(index)) + + await store.initialize() + + expect(store.getAll()).toHaveLength(2) + expect(store.get("task-1")).toBeDefined() + expect(store.get("task-2")).toBeDefined() + }) + }) + + describe("get()", () => { + it("returns undefined for non-existent task", async () => { + await store.initialize() + expect(store.get("non-existent")).toBeUndefined() + }) + + it("returns the item after upsert", async () => { + await store.initialize() + const item = makeHistoryItem({ id: "task-get" }) + await store.upsert(item) + expect(store.get("task-get")).toMatchObject({ id: "task-get" }) + }) + }) + + describe("getAll()", () => { + it("returns items sorted by ts descending", async () => { + await store.initialize() + + await store.upsert(makeHistoryItem({ id: "old", ts: 1000 })) + await store.upsert(makeHistoryItem({ id: "mid", ts: 2000 })) + await store.upsert(makeHistoryItem({ id: "new", ts: 3000 })) + + const all = store.getAll() + expect(all).toHaveLength(3) + expect(all[0].id).toBe("new") + expect(all[1].id).toBe("mid") + expect(all[2].id).toBe("old") + }) + }) + + describe("getByWorkspace()", () => { + it("filters by workspace path", async () => { + await store.initialize() + + await store.upsert(makeHistoryItem({ id: "ws-a-1", workspace: "/workspace-a" })) + await store.upsert(makeHistoryItem({ id: "ws-a-2", workspace: "/workspace-a" })) + await store.upsert(makeHistoryItem({ id: "ws-b-1", workspace: "/workspace-b" })) + + const wsA = store.getByWorkspace("/workspace-a") + expect(wsA).toHaveLength(2) + expect(wsA.every((item) => item.workspace === "/workspace-a")).toBe(true) + + const wsB = store.getByWorkspace("/workspace-b") + expect(wsB).toHaveLength(1) + expect(wsB[0].id).toBe("ws-b-1") + }) + }) + + describe("upsert()", () => { + it("writes per-task file and updates cache", async () => { + await store.initialize() + + const item = makeHistoryItem({ id: "upsert-task" }) + const result = await store.upsert(item) + + // Cache should be updated + expect(store.get("upsert-task")).toBeDefined() + expect(result.length).toBe(1) + + // Per-task file should exist + const filePath = path.join(tmpDir, "tasks", "upsert-task", GlobalFileNames.historyItem) + const raw = await fs.readFile(filePath, "utf8") + const written = JSON.parse(raw) + expect(written.id).toBe("upsert-task") + }) + + it("preserves existing metadata on partial updates (delegation fields)", async () => { + await store.initialize() + + const original = makeHistoryItem({ + id: "delegate-task", + status: "delegated", + delegatedToId: "child-1", + awaitingChildId: "child-1", + childIds: ["child-1"], + }) + await store.upsert(original) + + // Partial update that doesn't include delegation fields + const partialUpdate: HistoryItem = makeHistoryItem({ + id: "delegate-task", + tokensIn: 500, + tokensOut: 200, + }) + await store.upsert(partialUpdate) + + const result = store.get("delegate-task")! + expect(result.status).toBe("delegated") + expect(result.delegatedToId).toBe("child-1") + expect(result.awaitingChildId).toBe("child-1") + expect(result.childIds).toEqual(["child-1"]) + expect(result.tokensIn).toBe(500) + expect(result.tokensOut).toBe(200) + }) + + it("returns updated task history array", async () => { + await store.initialize() + + const item1 = makeHistoryItem({ id: "item-1", ts: 1000 }) + const item2 = makeHistoryItem({ id: "item-2", ts: 2000 }) + + await store.upsert(item1) + const result = await store.upsert(item2) + + expect(result).toHaveLength(2) + // Should be sorted by ts descending + expect(result[0].id).toBe("item-2") + expect(result[1].id).toBe("item-1") + }) + }) + + describe("delete()", () => { + it("removes per-task file and updates cache", async () => { + await store.initialize() + + const item = makeHistoryItem({ id: "del-task" }) + await store.upsert(item) + expect(store.get("del-task")).toBeDefined() + + await store.delete("del-task") + expect(store.get("del-task")).toBeUndefined() + expect(store.getAll()).toHaveLength(0) + }) + + it("handles deleting non-existent task gracefully", async () => { + await store.initialize() + await expect(store.delete("non-existent")).resolves.not.toThrow() + }) + }) + + describe("deleteMany()", () => { + it("removes multiple tasks in batch", async () => { + await store.initialize() + + await store.upsert(makeHistoryItem({ id: "batch-1" })) + await store.upsert(makeHistoryItem({ id: "batch-2" })) + await store.upsert(makeHistoryItem({ id: "batch-3" })) + expect(store.getAll()).toHaveLength(3) + + await store.deleteMany(["batch-1", "batch-3"]) + expect(store.getAll()).toHaveLength(1) + expect(store.get("batch-2")).toBeDefined() + }) + }) + + describe("reconcile()", () => { + it("detects tasks on disk missing from index", async () => { + await store.initialize() + + // Manually create a task directory with history_item.json + const tasksDir = path.join(tmpDir, "tasks") + const taskDir = path.join(tasksDir, "orphan-task") + await fs.mkdir(taskDir, { recursive: true }) + + const item = makeHistoryItem({ id: "orphan-task" }) + await fs.writeFile(path.join(taskDir, GlobalFileNames.historyItem), JSON.stringify(item)) + + // Reconcile should pick it up + await store.reconcile() + + expect(store.get("orphan-task")).toBeDefined() + expect(store.get("orphan-task")!.id).toBe("orphan-task") + }) + + it("removes tasks from cache that no longer exist on disk", async () => { + await store.initialize() + + const item = makeHistoryItem({ id: "removed-task" }) + await store.upsert(item) + expect(store.get("removed-task")).toBeDefined() + + // Remove the task directory from disk + const taskDir = path.join(tmpDir, "tasks", "removed-task") + await fs.rm(taskDir, { recursive: true, force: true }) + + // Reconcile should remove it from cache + await store.reconcile() + + expect(store.get("removed-task")).toBeUndefined() + }) + }) + + describe("concurrent upsert() calls are serialized", () => { + it("serializes concurrent writes so no entries are lost", async () => { + await store.initialize() + + // Fire 5 concurrent upserts + const promises = Array.from({ length: 5 }, (_, i) => + store.upsert(makeHistoryItem({ id: `concurrent-${i}`, ts: 1000 + i })), + ) + + await Promise.all(promises) + + const all = store.getAll() + expect(all).toHaveLength(5) + const ids = all.map((h) => h.id) + for (let i = 0; i < 5; i++) { + expect(ids).toContain(`concurrent-${i}`) + } + }) + + it("serializes interleaved upsert and delete", async () => { + await store.initialize() + + const item = makeHistoryItem({ id: "interleave-test", ts: 1000 }) + await store.upsert(item) + + // Concurrent update and delete of different items + const promise1 = store.upsert(makeHistoryItem({ id: "survivor", ts: 2000 })) + const promise2 = store.delete("interleave-test") + + await Promise.all([promise1, promise2]) + + expect(store.get("interleave-test")).toBeUndefined() + expect(store.get("survivor")).toBeDefined() + }) + }) + + describe("migrateFromGlobalState()", () => { + it("writes history_item.json for tasks with existing directories", async () => { + await store.initialize() + + const tasksDir = path.join(tmpDir, "tasks") + + // Create task directories (simulating existing tasks) + await fs.mkdir(path.join(tasksDir, "legacy-1"), { recursive: true }) + await fs.mkdir(path.join(tasksDir, "legacy-2"), { recursive: true }) + + const items = [ + makeHistoryItem({ id: "legacy-1", task: "Legacy task 1" }), + makeHistoryItem({ id: "legacy-2", task: "Legacy task 2" }), + makeHistoryItem({ id: "legacy-orphan", task: "Orphaned task" }), // No directory + ] + + await store.migrateFromGlobalState(items) + + // Should have migrated 2 items (skipping orphan) + expect(store.get("legacy-1")).toBeDefined() + expect(store.get("legacy-2")).toBeDefined() + expect(store.get("legacy-orphan")).toBeUndefined() + }) + + it("does not overwrite existing per-task files", async () => { + await store.initialize() + + const tasksDir = path.join(tmpDir, "tasks") + const taskDir = path.join(tasksDir, "existing-task") + await fs.mkdir(taskDir, { recursive: true }) + + // Write an existing history_item.json with specific data + const existingItem = makeHistoryItem({ + id: "existing-task", + task: "Original task text", + tokensIn: 999, + }) + await fs.writeFile(path.join(taskDir, GlobalFileNames.historyItem), JSON.stringify(existingItem)) + + // Try to migrate with different data + const migratedItem = makeHistoryItem({ + id: "existing-task", + task: "Different task text", + tokensIn: 1, + }) + await store.migrateFromGlobalState([migratedItem]) + + // Existing file should not be overwritten + const raw = await fs.readFile(path.join(taskDir, GlobalFileNames.historyItem), "utf8") + const persisted = JSON.parse(raw) + expect(persisted.task).toBe("Original task text") + expect(persisted.tokensIn).toBe(999) + }) + + it("is idempotent (can be called multiple times safely)", async () => { + await store.initialize() + + const tasksDir = path.join(tmpDir, "tasks") + await fs.mkdir(path.join(tasksDir, "idem-task"), { recursive: true }) + + const item = makeHistoryItem({ id: "idem-task" }) + + await store.migrateFromGlobalState([item]) + await store.migrateFromGlobalState([item]) // Second call + + expect(store.get("idem-task")).toBeDefined() + }) + }) + + describe("flushIndex()", () => { + it("writes index to disk on flush", async () => { + await store.initialize() + + await store.upsert(makeHistoryItem({ id: "flush-task" })) + await store.flushIndex() + + const indexPath = path.join(tmpDir, "tasks", GlobalFileNames.historyIndex) + const raw = await fs.readFile(indexPath, "utf8") + const index = JSON.parse(raw) + + expect(index.version).toBe(1) + expect(index.entries).toHaveLength(1) + expect(index.entries[0].id).toBe("flush-task") + }) + }) + + describe("dispose()", () => { + it("flushes index on dispose", async () => { + await store.initialize() + + await store.upsert(makeHistoryItem({ id: "dispose-task" })) + store.dispose() + + // Give the flush a moment to complete + await new Promise((resolve) => setTimeout(resolve, 100)) + + const indexPath = path.join(tmpDir, "tasks", GlobalFileNames.historyIndex) + const raw = await fs.readFile(indexPath, "utf8") + const index = JSON.parse(raw) + expect(index.entries).toHaveLength(1) + }) + }) + + describe("invalidate()", () => { + it("re-reads a task from disk", async () => { + await store.initialize() + + const item = makeHistoryItem({ id: "invalidate-task", tokensIn: 100 }) + await store.upsert(item) + + // Manually update the file on disk + const filePath = path.join(tmpDir, "tasks", "invalidate-task", GlobalFileNames.historyItem) + const updated = { ...item, tokensIn: 999 } + await fs.writeFile(filePath, JSON.stringify(updated)) + + await store.invalidate("invalidate-task") + + expect(store.get("invalidate-task")!.tokensIn).toBe(999) + }) + + it("removes item from cache if file no longer exists", async () => { + await store.initialize() + + const item = makeHistoryItem({ id: "gone-task" }) + await store.upsert(item) + + // Delete the file + const filePath = path.join(tmpDir, "tasks", "gone-task", GlobalFileNames.historyItem) + await fs.unlink(filePath) + + await store.invalidate("gone-task") + + expect(store.get("gone-task")).toBeUndefined() + }) + }) +}) diff --git a/src/core/task-persistence/index.ts b/src/core/task-persistence/index.ts index c8656002bde..115711e6fd2 100644 --- a/src/core/task-persistence/index.ts +++ b/src/core/task-persistence/index.ts @@ -1,3 +1,4 @@ export { type ApiMessage, readApiMessages, saveApiMessages } from "./apiMessages" export { readTaskMessages, saveTaskMessages } from "./taskMessages" export { taskMetadata } from "./taskMetadata" +export { TaskHistoryStore } from "./TaskHistoryStore" diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index bb9199a65c2..460581c5f84 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -97,7 +97,7 @@ import { Task } from "../task/Task" import { webviewMessageHandler } from "./webviewMessageHandler" import type { ClineMessage, TodoItem } from "@roo-code/types" -import { readApiMessages, saveApiMessages, saveTaskMessages } from "../task-persistence" +import { readApiMessages, saveApiMessages, saveTaskMessages, TaskHistoryStore } from "../task-persistence" import { readTaskMessages } from "../task-persistence/taskMessages" import { getNonce } from "./getNonce" import { getUri } from "./getUri" @@ -150,7 +150,8 @@ export class ClineProvider private _disposed = false private recentTasksCache?: string[] - private taskHistoryWriteLock: Promise = Promise.resolve() + public readonly taskHistoryStore: TaskHistoryStore + private taskHistoryStoreInitialized = false private pendingOperations: Map = new Map() private static readonly PENDING_OPERATION_TIMEOUT_MS = 30000 // 30 seconds @@ -185,6 +186,12 @@ export class ClineProvider this.mdmService = mdmService this.updateGlobalState("codebaseIndexModels", EMBEDDING_MODEL_PROFILES) + // Initialize the 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}`) + }) + // Start configuration loading (which might trigger indexing) in the background. // Don't await, allowing activation to continue immediately. @@ -314,6 +321,35 @@ export class ClineProvider } } + /** + * Initialize the TaskHistoryStore and migrate from globalState if needed. + */ + private async initializeTaskHistoryStore(): Promise { + try { + await this.taskHistoryStore.initialize() + + // Migration: backfill per-task files from globalState on first run + const migrationKey = "taskHistoryMigratedToFiles" + const alreadyMigrated = this.context.globalState.get(migrationKey) + + if (!alreadyMigrated) { + const legacyHistory = this.context.globalState.get("taskHistory") ?? [] + + if (legacyHistory.length > 0) { + this.log(`[initializeTaskHistoryStore] Migrating ${legacyHistory.length} entries from globalState`) + await this.taskHistoryStore.migrateFromGlobalState(legacyHistory) + } + + await this.context.globalState.update(migrationKey, true) + this.log("[initializeTaskHistoryStore] Migration complete") + } + + this.taskHistoryStoreInitialized = true + } catch (error) { + this.log(`[initializeTaskHistoryStore] Error: ${error instanceof Error ? error.message : String(error)}`) + } + } + /** * Override EventEmitter's on method to match TaskProviderLike interface */ @@ -667,6 +703,7 @@ export class ClineProvider this.skillsManager = undefined this.marketplaceManager?.cleanup() this.customModesManager?.dispose() + this.taskHistoryStore.dispose() this.log("Disposed all disposables") ClineProvider.activeInstances.delete(this) @@ -1344,12 +1381,12 @@ export class ClineProvider try { // Update the task history with the new mode first. - const history = this.getGlobalState("taskHistory") ?? [] - const taskHistoryItem = history.find((item) => item.id === task.taskId) + const taskHistoryItem = + this.taskHistoryStore.get(task.taskId) ?? + (this.getGlobalState("taskHistory") ?? []).find((item) => item.id === task.taskId) if (taskHistoryItem) { - taskHistoryItem.mode = newMode - await this.updateTaskHistory(taskHistoryItem) + await this.updateTaskHistory({ ...taskHistoryItem, mode: newMode }) } // Only update the task's mode after successful persistence. @@ -1563,8 +1600,9 @@ export class ClineProvider // been persisted into taskHistory (it will be captured on the next save). task.setTaskApiConfigName(apiConfigName) - const history = this.getGlobalState("taskHistory") ?? [] - const taskHistoryItem = history.find((item) => item.id === task.taskId) + const taskHistoryItem = + this.taskHistoryStore.get(task.taskId) ?? + (this.getGlobalState("taskHistory") ?? []).find((item) => item.id === task.taskId) if (taskHistoryItem) { await this.updateTaskHistory({ ...taskHistoryItem, apiConfigName }) @@ -1723,8 +1761,8 @@ export class ClineProvider uiMessagesFilePath: string apiConversationHistory: Anthropic.MessageParam[] }> { - const history = this.getGlobalState("taskHistory") ?? [] - const historyItem = history.find((item) => item.id === id) + const historyItem = + this.taskHistoryStore.get(id) ?? (this.getGlobalState("taskHistory") ?? []).find((item) => item.id === id) if (!historyItem) { throw new Error("Task not found") @@ -1856,12 +1894,12 @@ export class ClineProvider } // Delete all tasks from state in one batch - await this.withTaskHistoryLock(async () => { - const taskHistory = this.getGlobalState("taskHistory") ?? [] - const updatedTaskHistory = taskHistory.filter((task) => !allIdsToDelete.includes(task.id)) - await this.updateGlobalState("taskHistory", updatedTaskHistory) - this.recentTasksCache = undefined - }) + await this.taskHistoryStore.deleteMany(allIdsToDelete) + this.recentTasksCache = undefined + + // Write-through to globalState during transition period + const remainingHistory = this.taskHistoryStore.getAll() + await this.updateGlobalState("taskHistory", remainingHistory) // Delete associated shadow repositories or branches and task directories const globalStorageDir = this.contextProxy.globalStorageUri.fsPath @@ -1902,12 +1940,13 @@ export class ClineProvider } async deleteTaskFromState(id: string) { - await this.withTaskHistoryLock(async () => { - const taskHistory = this.getGlobalState("taskHistory") ?? [] - const updatedTaskHistory = taskHistory.filter((task) => task.id !== id) - await this.updateGlobalState("taskHistory", updatedTaskHistory) - this.recentTasksCache = undefined - }) + await this.taskHistoryStore.delete(id) + this.recentTasksCache = undefined + + // Write-through to globalState during transition period + const remainingHistory = this.taskHistoryStore.getAll() + await this.updateGlobalState("taskHistory", remainingHistory) + await this.postStateToWebview() } @@ -2206,14 +2245,12 @@ export class ClineProvider autoCondenseContextPercent: autoCondenseContextPercent ?? 100, uriScheme: vscode.env.uriScheme, currentTaskItem: this.getCurrentTask()?.taskId - ? (taskHistory || []).find((item: HistoryItem) => item.id === this.getCurrentTask()?.taskId) + ? this.taskHistoryStore.get(this.getCurrentTask()!.taskId) : undefined, clineMessages: this.getCurrentTask()?.clineMessages || [], currentTaskTodos: this.getCurrentTask()?.todoList || [], messageQueue: this.getCurrentTask()?.messageQueueService?.messages, - taskHistory: (taskHistory || []) - .filter((item: HistoryItem) => item.ts && item.task) - .sort((a: HistoryItem, b: HistoryItem) => b.ts - a.ts), + taskHistory: this.taskHistoryStore.getAll().filter((item: HistoryItem) => item.ts && item.task), soundEnabled: soundEnabled ?? false, ttsEnabled: ttsEnabled ?? false, ttsSpeed: ttsSpeed ?? 1.0, @@ -2443,7 +2480,7 @@ export class ClineProvider allowedMaxCost: stateValues.allowedMaxCost, autoCondenseContext: stateValues.autoCondenseContext ?? true, autoCondenseContextPercent: stateValues.autoCondenseContextPercent ?? 100, - taskHistory: stateValues.taskHistory ?? [], + taskHistory: this.taskHistoryStore.getAll(), allowedCommands: stateValues.allowedCommands, deniedCommands: stateValues.deniedCommands, soundEnabled: stateValues.soundEnabled ?? false, @@ -2552,69 +2589,44 @@ export class ClineProvider } } - /** - * Serializes all read-modify-write operations on taskHistory to prevent - * concurrent interleaving that can cause entries to vanish. - */ - private withTaskHistoryLock(fn: () => Promise): Promise { - const result = this.taskHistoryWriteLock.then(fn, fn) // run even if previous write errored - this.taskHistoryWriteLock = result.then( - () => {}, - () => {}, - ) // swallow for chain continuity - return result - } - /** * Updates a task in the task history and optionally broadcasts the updated history to the webview. + * Now delegates to TaskHistoryStore for per-task file persistence. + * * @param item The history item to update or add * @param options.broadcast Whether to broadcast the updated history to the webview (default: true) * @returns The updated task history array */ async updateTaskHistory(item: HistoryItem, options: { broadcast?: boolean } = {}): Promise { - return this.withTaskHistoryLock(async () => { - const { broadcast = true } = options - const history = (this.getGlobalState("taskHistory") as HistoryItem[] | undefined) || [] - const existingItemIndex = history.findIndex((h) => h.id === item.id) - const wasExisting = existingItemIndex !== -1 - - if (wasExisting) { - // Preserve existing metadata (e.g., delegation fields) unless explicitly overwritten. - // This prevents loss of status/awaitingChildId/delegatedToId when tasks are reopened, - // terminated, or when routine message persistence occurs. - history[existingItemIndex] = { - ...history[existingItemIndex], - ...item, - } - } else { - history.push(item) - } + const { broadcast = true } = options - await this.updateGlobalState("taskHistory", history) - this.recentTasksCache = undefined + const history = await this.taskHistoryStore.upsert(item) + this.recentTasksCache = undefined - // Broadcast the updated history to the webview if requested. - // Prefer per-item updates to avoid repeatedly cloning/sending the full history. - if (broadcast && this.isViewLaunched) { - const updatedItem = wasExisting ? history[existingItemIndex] : item - await this.postMessageToWebview({ type: "taskHistoryItemUpdated", taskHistoryItem: updatedItem }) - } + // Write-through to globalState during transition period for backward compatibility + await this.updateGlobalState("taskHistory", history) - return history - }) + // Broadcast the updated history to the webview if requested. + // Prefer per-item updates to avoid repeatedly cloning/sending the full history. + if (broadcast && this.isViewLaunched) { + const updatedItem = this.taskHistoryStore.get(item.id) ?? item + await this.postMessageToWebview({ type: "taskHistoryItemUpdated", taskHistoryItem: updatedItem }) + } + + return history } /** * Broadcasts a task history update to the webview. * This sends a lightweight message with just the task history, rather than the full state. - * @param history The task history to broadcast (if not provided, reads from global state) + * @param history The task history to broadcast (if not provided, reads from the store) */ public async broadcastTaskHistoryUpdate(history?: HistoryItem[]): Promise { if (!this.isViewLaunched) { return } - const taskHistory = history ?? (this.getGlobalState("taskHistory") as HistoryItem[] | undefined) ?? [] + const taskHistory = history ?? this.taskHistoryStore.getAll() // Sort and filter the history the same way as getStateToPostToWebview const sortedHistory = taskHistory @@ -2865,7 +2877,7 @@ export class ClineProvider return this.recentTasksCache } - const history = this.getGlobalState("taskHistory") ?? [] + const history = this.taskHistoryStore.getAll() const workspaceTasks: HistoryItem[] = [] for (const item of history) { diff --git a/src/core/webview/__tests__/ClineProvider.sticky-mode.spec.ts b/src/core/webview/__tests__/ClineProvider.sticky-mode.spec.ts index 9e4f2fab3ad..f24cee0786c 100644 --- a/src/core/webview/__tests__/ClineProvider.sticky-mode.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.sticky-mode.spec.ts @@ -165,10 +165,23 @@ vi.mock("fs/promises", () => ({ mkdir: vi.fn().mockResolvedValue(undefined), writeFile: vi.fn().mockResolvedValue(undefined), readFile: vi.fn().mockResolvedValue(""), + readdir: vi.fn().mockResolvedValue([]), unlink: vi.fn().mockResolvedValue(undefined), rmdir: vi.fn().mockResolvedValue(undefined), + access: vi.fn().mockResolvedValue(undefined), + rm: vi.fn().mockResolvedValue(undefined), })) +vi.mock("../../../utils/storage", async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + getStorageBasePath: vi.fn().mockImplementation((defaultPath: string) => defaultPath), + getSettingsDirectoryPath: vi.fn().mockResolvedValue("/test/settings/path"), + getTaskDirectoryPath: vi.fn().mockResolvedValue("/test/task/path"), + } +}) + vi.mock("@roo-code/telemetry", () => ({ TelemetryService: { hasInstance: vi.fn().mockReturnValue(true), @@ -191,7 +204,7 @@ describe("ClineProvider - Sticky Mode", () => { let mockWebviewView: vscode.WebviewView let mockPostMessage: any - beforeEach(() => { + beforeEach(async () => { vi.clearAllMocks() if (!TelemetryService.hasInstance()) { @@ -268,6 +281,9 @@ describe("ClineProvider - Sticky Mode", () => { provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext)) + // Wait for the async TaskHistoryStore initialization to complete + await new Promise((resolve) => setTimeout(resolve, 10)) + // Mock getMcpHub method provider.getMcpHub = vi.fn().mockReturnValue({ listTools: vi.fn().mockResolvedValue([]), diff --git a/src/core/webview/__tests__/ClineProvider.sticky-profile.spec.ts b/src/core/webview/__tests__/ClineProvider.sticky-profile.spec.ts index 2f29d79d0ef..0bea9b1c368 100644 --- a/src/core/webview/__tests__/ClineProvider.sticky-profile.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.sticky-profile.spec.ts @@ -166,10 +166,23 @@ vi.mock("fs/promises", () => ({ mkdir: vi.fn().mockResolvedValue(undefined), writeFile: vi.fn().mockResolvedValue(undefined), readFile: vi.fn().mockResolvedValue(""), + readdir: vi.fn().mockResolvedValue([]), unlink: vi.fn().mockResolvedValue(undefined), rmdir: vi.fn().mockResolvedValue(undefined), + access: vi.fn().mockResolvedValue(undefined), + rm: vi.fn().mockResolvedValue(undefined), })) +vi.mock("../../../utils/storage", async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + getStorageBasePath: vi.fn().mockImplementation((defaultPath: string) => defaultPath), + getSettingsDirectoryPath: vi.fn().mockResolvedValue("/test/settings/path"), + getTaskDirectoryPath: vi.fn().mockResolvedValue("/test/task/path"), + } +}) + vi.mock("@roo-code/telemetry", () => ({ TelemetryService: { hasInstance: vi.fn().mockReturnValue(true), @@ -192,7 +205,7 @@ describe("ClineProvider - Sticky Provider Profile", () => { let mockWebviewView: vscode.WebviewView let mockPostMessage: any - beforeEach(() => { + beforeEach(async () => { vi.clearAllMocks() taskIdCounter = 0 @@ -270,6 +283,9 @@ describe("ClineProvider - Sticky Provider Profile", () => { provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext)) + // Wait for the async TaskHistoryStore initialization to complete + await new Promise((resolve) => setTimeout(resolve, 10)) + // Mock getMcpHub method provider.getMcpHub = vi.fn().mockReturnValue({ listTools: vi.fn().mockResolvedValue([]), @@ -301,20 +317,16 @@ describe("ClineProvider - Sticky Provider Profile", () => { // Add task to provider stack await provider.addClineToStack(mockTask as any) - // Mock getGlobalState to return task history - vi.spyOn(provider as any, "getGlobalState").mockReturnValue([ - { - id: mockTask.taskId, - ts: Date.now(), - task: "Test task", - number: 1, - tokensIn: 0, - tokensOut: 0, - cacheWrites: 0, - cacheReads: 0, - totalCost: 0, - }, - ]) + // Populate the store so persistStickyProviderProfileToCurrentTask finds the task + await provider.taskHistoryStore.upsert({ + id: mockTask.taskId, + ts: Date.now(), + task: "Test task", + number: 1, + tokensIn: 0, + tokensOut: 0, + totalCost: 0, + }) // Mock updateTaskHistory to track calls const updateTaskHistorySpy = vi @@ -608,20 +620,16 @@ describe("ClineProvider - Sticky Provider Profile", () => { updateApiConfiguration: vi.fn(), } - // Mock getGlobalState to return task history with our task - vi.spyOn(provider as any, "getGlobalState").mockReturnValue([ - { - id: mockTask.taskId, - ts: Date.now(), - task: "Test task", - number: 1, - tokensIn: 0, - tokensOut: 0, - cacheWrites: 0, - cacheReads: 0, - totalCost: 0, - }, - ]) + // Populate the store so persistStickyProviderProfileToCurrentTask finds the task + await provider.taskHistoryStore.upsert({ + id: mockTask.taskId, + ts: Date.now(), + task: "Test task", + number: 1, + tokensIn: 0, + tokensOut: 0, + totalCost: 0, + }) // Mock updateTaskHistory to capture the updated history item let updatedHistoryItem: any @@ -720,7 +728,10 @@ describe("ClineProvider - Sticky Provider Profile", () => { }, ] - vi.spyOn(provider as any, "getGlobalState").mockReturnValue(taskHistory) + // Populate the store + for (const item of taskHistory) { + await provider.taskHistoryStore.upsert(item as any) + } // Mock updateTaskHistory vi.spyOn(provider, "updateTaskHistory").mockImplementation((item) => { @@ -776,20 +787,16 @@ describe("ClineProvider - Sticky Provider Profile", () => { // Add task to provider stack await provider.addClineToStack(mockTask as any) - // Mock getGlobalState - vi.spyOn(provider as any, "getGlobalState").mockReturnValue([ - { - id: mockTask.taskId, - ts: Date.now(), - task: "Test task", - number: 1, - tokensIn: 0, - tokensOut: 0, - cacheWrites: 0, - cacheReads: 0, - totalCost: 0, - }, - ]) + // Populate the store + await provider.taskHistoryStore.upsert({ + id: mockTask.taskId, + ts: Date.now(), + task: "Test task", + number: 1, + tokensIn: 0, + tokensOut: 0, + totalCost: 0, + }) // Mock updateTaskHistory to throw error vi.spyOn(provider, "updateTaskHistory").mockRejectedValue(new Error("Save failed")) diff --git a/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts b/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts index 72a6f839608..7fac36f59e6 100644 --- a/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts @@ -17,8 +17,11 @@ vi.mock("fs/promises", () => ({ mkdir: vi.fn().mockResolvedValue(undefined), writeFile: vi.fn().mockResolvedValue(undefined), readFile: vi.fn().mockResolvedValue(""), + readdir: vi.fn().mockResolvedValue([]), unlink: vi.fn().mockResolvedValue(undefined), rmdir: vi.fn().mockResolvedValue(undefined), + access: vi.fn().mockResolvedValue(undefined), + rm: vi.fn().mockResolvedValue(undefined), })) vi.mock("axios", () => ({ @@ -44,6 +47,11 @@ vi.mock("../../../utils/storage", () => ({ getSettingsDirectoryPath: vi.fn().mockResolvedValue("/test/settings/path"), getTaskDirectoryPath: vi.fn().mockResolvedValue("/test/task/path"), getGlobalStoragePath: vi.fn().mockResolvedValue("/test/storage/path"), + getStorageBasePath: vi.fn().mockImplementation((defaultPath: string) => defaultPath), +})) + +vi.mock("../../../utils/safeWriteJson", () => ({ + safeWriteJson: vi.fn().mockResolvedValue(undefined), })) vi.mock("@modelcontextprotocol/sdk/types.js", () => ({ @@ -239,7 +247,7 @@ describe("ClineProvider Task History Synchronization", () => { let mockPostMessage: ReturnType let taskHistoryState: HistoryItem[] - beforeEach(() => { + beforeEach(async () => { vi.clearAllMocks() if (!TelemetryService.hasInstance()) { @@ -316,6 +324,10 @@ describe("ClineProvider Task History Synchronization", () => { provider = new ClineProvider(mockContext, mockOutputChannel, "sidebar", new ContextProxy(mockContext)) + // Wait for the async TaskHistoryStore initialization to complete + // (fire-and-forget from the constructor; microtasks need to flush) + await new Promise((resolve) => setTimeout(resolve, 10)) + // Mock the custom modes manager ;(provider as any).customModesManager = { updateCustomMode: vi.fn().mockResolvedValue(undefined), @@ -582,18 +594,14 @@ describe("ClineProvider Task History Synchronization", () => { expect(sentHistory[0].id).toBe("valid") }) - it("reads from global state when no history is provided", async () => { + it("reads from store when no history is provided", async () => { await provider.resolveWebviewView(mockWebviewView) provider.isViewLaunched = true - // Set up task history in global state + // Populate the store with an item const now = Date.now() - const stateHistory: HistoryItem[] = [createHistoryItem({ id: "from-state", ts: now, task: "State task" })] - - // Update the mock to return our history - ;(mockContext.globalState.get as ReturnType).mockImplementation((key: string) => { - if (key === "taskHistory") return stateHistory - return undefined + await provider.updateTaskHistory(createHistoryItem({ id: "from-store", ts: now, task: "Store task" }), { + broadcast: false, }) // Clear previous calls @@ -605,8 +613,8 @@ describe("ClineProvider Task History Synchronization", () => { const call = calls.find((c) => c[0]?.type === "taskHistoryUpdated") const sentHistory = call?.[0]?.taskHistory as HistoryItem[] - expect(sentHistory.length).toBe(1) - expect(sentHistory[0].id).toBe("from-state") + expect(sentHistory.length).toBeGreaterThanOrEqual(1) + expect(sentHistory.some((item) => item.id === "from-store")).toBe(true) }) }) @@ -615,13 +623,18 @@ describe("ClineProvider Task History Synchronization", () => { await provider.resolveWebviewView(mockWebviewView) const now = Date.now() - const multiWorkspaceHistory: HistoryItem[] = [ + + // Populate the store with multi-workspace items + await provider.updateTaskHistory( createHistoryItem({ id: "ws1-task", ts: now, task: "Workspace 1 task", workspace: "/path/to/workspace1", }), + { broadcast: false }, + ) + await provider.updateTaskHistory( createHistoryItem({ id: "ws2-task", ts: now - 1000, @@ -629,6 +642,9 @@ describe("ClineProvider Task History Synchronization", () => { workspace: "/path/to/workspace2", number: 2, }), + { broadcast: false }, + ) + await provider.updateTaskHistory( createHistoryItem({ id: "ws3-task", ts: now - 2000, @@ -636,13 +652,8 @@ describe("ClineProvider Task History Synchronization", () => { workspace: "/different/workspace", number: 3, }), - ] - - // Update the mock to return multi-workspace history - ;(mockContext.globalState.get as ReturnType).mockImplementation((key: string) => { - if (key === "taskHistory") return multiWorkspaceHistory - return undefined - }) + { broadcast: false }, + ) const state = await provider.getStateToPostToWebview() @@ -700,7 +711,7 @@ describe("ClineProvider Task History Synchronization", () => { it("does not block subsequent writes when a previous write errors", async () => { await provider.resolveWebviewView(mockWebviewView) - // Temporarily make updateGlobalState throw + // Temporarily make the globalState write-through throw const origUpdateGlobalState = (provider as any).updateGlobalState.bind(provider) let callCount = 0 ;(provider as any).updateGlobalState = vi.fn().mockImplementation((...args: unknown[]) => { @@ -711,13 +722,13 @@ describe("ClineProvider Task History Synchronization", () => { return origUpdateGlobalState(...args) }) - // First call should fail + // First call should fail (due to write-through to globalState) const item1 = createHistoryItem({ id: "fail-item", task: "Fail" }) await expect(provider.updateTaskHistory(item1, { broadcast: false })).rejects.toThrow( "simulated write failure", ) - // Second call should still succeed (lock not stuck) + // Second call should still succeed (store lock not stuck) const item2 = createHistoryItem({ id: "ok-item", task: "OK" }) const result = await provider.updateTaskHistory(item2, { broadcast: false }) expect(result.some((h) => h.id === "ok-item")).toBe(true) diff --git a/src/shared/globalFileNames.ts b/src/shared/globalFileNames.ts index 98b48485f06..0b54ff6809c 100644 --- a/src/shared/globalFileNames.ts +++ b/src/shared/globalFileNames.ts @@ -4,4 +4,6 @@ export const GlobalFileNames = { mcpSettings: "mcp_settings.json", customModes: "custom_modes.yaml", taskMetadata: "task_metadata.json", + historyItem: "history_item.json", + historyIndex: "_index.json", } From 8e5f0b240ffa1afd47874ef922ecc059298affc1 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Mon, 16 Feb 2026 18:31:41 +0000 Subject: [PATCH 2/4] fix: address review feedback - reconcile lock, init promise, write-through serialization - reconcile() now runs through withLock() to prevent interleaving with upsert/delete at async boundaries - Added initialized promise so callers can await store readiness before reading (getStateToPostToWebview now awaits it) - Write-through to globalState now happens inside the store lock via onWrite callback, preventing concurrent call races on the transition period fallback - Removed separate updateGlobalState("taskHistory") calls from ClineProvider since the onWrite callback handles it serialized --- src/core/task-persistence/TaskHistoryStore.ts | 141 ++++++++++++------ src/core/webview/ClineProvider.ts | 23 ++- 2 files changed, 105 insertions(+), 59 deletions(-) diff --git a/src/core/task-persistence/TaskHistoryStore.ts b/src/core/task-persistence/TaskHistoryStore.ts index e542d27853d..4157d8b9fbb 100644 --- a/src/core/task-persistence/TaskHistoryStore.ts +++ b/src/core/task-persistence/TaskHistoryStore.ts @@ -29,8 +29,21 @@ interface HistoryIndex { * on per-task file writes. Within a single extension host process, * an in-process write lock serializes mutations. */ +/** + * Options for TaskHistoryStore constructor. + */ +export interface TaskHistoryStoreOptions { + /** + * Optional callback invoked inside the write lock after each mutation + * (upsert, delete, deleteMany). Used for serialized write-through to + * globalState during the transition period. + */ + onWrite?: (items: HistoryItem[]) => Promise +} + export class TaskHistoryStore { private readonly globalStoragePath: string + private readonly onWrite?: (items: HistoryItem[]) => Promise private cache: Map = new Map() private writeLock: Promise = Promise.resolve() private indexWriteTimer: ReturnType | null = null @@ -38,14 +51,25 @@ export class TaskHistoryStore { private reconcileTimer: ReturnType | null = null private disposed = false + /** + * Promise that resolves when initialization is complete. + * Callers can await this to ensure the store is ready before reading. + */ + public readonly initialized: Promise + private resolveInitialized!: () => void + /** Debounce window for index writes in milliseconds. */ private static readonly INDEX_WRITE_DEBOUNCE_MS = 2000 /** Periodic reconciliation interval in milliseconds. */ private static readonly RECONCILE_INTERVAL_MS = 5 * 60 * 1000 - constructor(globalStoragePath: string) { + constructor(globalStoragePath: string, options?: TaskHistoryStoreOptions) { this.globalStoragePath = globalStoragePath + this.onWrite = options?.onWrite + this.initialized = new Promise((resolve) => { + this.resolveInitialized = resolve + }) } // ────────────────────────────── Lifecycle ────────────────────────────── @@ -54,20 +78,25 @@ export class TaskHistoryStore { * Load index, reconcile if needed, start watchers. */ async initialize(): Promise { - const tasksDir = await this.getTasksDir() - await fs.mkdir(tasksDir, { recursive: true }) + try { + const tasksDir = await this.getTasksDir() + await fs.mkdir(tasksDir, { recursive: true }) - // 1. Load existing index into the cache - await this.loadIndex() + // 1. Load existing index into the cache + await this.loadIndex() - // 2. Reconcile cache against actual task directories on disk - await this.reconcile() + // 2. Reconcile cache against actual task directories on disk + await this.reconcile() - // 3. Start fs.watch for cross-instance reactivity - this.startWatcher() + // 3. Start fs.watch for cross-instance reactivity + this.startWatcher() - // 4. Start periodic reconciliation as a defensive fallback - this.startPeriodicReconciliation() + // 4. Start periodic reconciliation as a defensive fallback + this.startPeriodicReconciliation() + } finally { + // Mark initialization as complete so callers awaiting `initialized` can proceed + this.resolveInitialized() + } } /** @@ -144,7 +173,14 @@ export class TaskHistoryStore { // Schedule debounced index write this.scheduleIndexWrite() - return this.getAll() + const all = this.getAll() + + // Call onWrite callback inside the lock for serialized write-through + if (this.onWrite) { + await this.onWrite(all) + } + + return all }) } @@ -164,6 +200,11 @@ export class TaskHistoryStore { } this.scheduleIndexWrite() + + // Call onWrite callback inside the lock for serialized write-through + if (this.onWrite) { + await this.onWrite(this.getAll()) + } }) } @@ -184,6 +225,11 @@ export class TaskHistoryStore { } this.scheduleIndexWrite() + + // Call onWrite callback inside the lock for serialized write-through + if (this.onWrite) { + await this.onWrite(this.getAll()) + } }) } @@ -196,48 +242,51 @@ export class TaskHistoryStore { * - Tasks in cache but missing from disk: remove */ async reconcile(): Promise { - const tasksDir = await this.getTasksDir() - - let dirEntries: string[] - try { - dirEntries = await fs.readdir(tasksDir) - } catch { - return // tasks dir doesn't exist yet - } - - // Filter out the index file and hidden files - const taskDirNames = dirEntries.filter((name) => !name.startsWith("_") && !name.startsWith(".")) + // Run through the write lock to prevent interleaving with upsert/delete + return this.withLock(async () => { + const tasksDir = await this.getTasksDir() - const onDiskIds = new Set(taskDirNames) - const cacheIds = new Set(this.cache.keys()) - let changed = false + let dirEntries: string[] + try { + dirEntries = await fs.readdir(tasksDir) + } catch { + return // tasks dir doesn't exist yet + } - // Tasks on disk but not in cache: read their history_item.json - for (const taskId of onDiskIds) { - if (!cacheIds.has(taskId)) { - try { - const item = await this.readTaskFile(taskId) - if (item) { - this.cache.set(taskId, item) - changed = true + // Filter out the index file and hidden files + const taskDirNames = dirEntries.filter((name) => !name.startsWith("_") && !name.startsWith(".")) + + const onDiskIds = new Set(taskDirNames) + const cacheIds = new Set(this.cache.keys()) + let changed = false + + // Tasks on disk but not in cache: read their history_item.json + for (const taskId of onDiskIds) { + if (!cacheIds.has(taskId)) { + try { + const item = await this.readTaskFile(taskId) + if (item) { + this.cache.set(taskId, item) + changed = true + } + } catch { + // Corrupted or missing file, skip } - } catch { - // Corrupted or missing file, skip } } - } - // Tasks in cache but not on disk: remove from cache - for (const taskId of cacheIds) { - if (!onDiskIds.has(taskId)) { - this.cache.delete(taskId) - changed = true + // Tasks in cache but not on disk: remove from cache + for (const taskId of cacheIds) { + if (!onDiskIds.has(taskId)) { + this.cache.delete(taskId) + changed = true + } } - } - if (changed) { - this.scheduleIndexWrite() - } + if (changed) { + this.scheduleIndexWrite() + } + }) } // ────────────────────────────── Cache invalidation ────────────────────────────── diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 460581c5f84..8281da85e7c 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -186,8 +186,13 @@ export class ClineProvider this.mdmService = mdmService this.updateGlobalState("codebaseIndexModels", EMBEDDING_MODEL_PROFILES) - // Initialize the per-task file-based history store. - this.taskHistoryStore = new TaskHistoryStore(this.contextProxy.globalStorageUri.fsPath) + // Initialize the per-task file-based history store with write-through callback. + this.taskHistoryStore = new TaskHistoryStore(this.contextProxy.globalStorageUri.fsPath, { + onWrite: async (items) => { + // Write-through to globalState inside the store's lock for transition period + await this.updateGlobalState("taskHistory", items) + }, + }) this.initializeTaskHistoryStore().catch((error) => { this.log(`Failed to initialize TaskHistoryStore: ${error}`) }) @@ -1897,10 +1902,6 @@ export class ClineProvider await this.taskHistoryStore.deleteMany(allIdsToDelete) this.recentTasksCache = undefined - // Write-through to globalState during transition period - const remainingHistory = this.taskHistoryStore.getAll() - await this.updateGlobalState("taskHistory", remainingHistory) - // Delete associated shadow repositories or branches and task directories const globalStorageDir = this.contextProxy.globalStorageUri.fsPath const workspaceDir = this.cwd @@ -1943,10 +1944,6 @@ export class ClineProvider await this.taskHistoryStore.delete(id) this.recentTasksCache = undefined - // Write-through to globalState during transition period - const remainingHistory = this.taskHistoryStore.getAll() - await this.updateGlobalState("taskHistory", remainingHistory) - await this.postStateToWebview() } @@ -2113,6 +2110,9 @@ export class ClineProvider } async getStateToPostToWebview(): Promise { + // Ensure the store is initialized before reading task history + await this.taskHistoryStore.initialized + const { apiConfiguration, lastShownAnnouncementId, @@ -2603,9 +2603,6 @@ export class ClineProvider const history = await this.taskHistoryStore.upsert(item) this.recentTasksCache = undefined - // Write-through to globalState during transition period for backward compatibility - await this.updateGlobalState("taskHistory", history) - // Broadcast the updated history to the webview if requested. // Prefer per-item updates to avoid repeatedly cloning/sending the full history. if (broadcast && this.isViewLaunched) { From 6b80bb81acb831e17c9e74109d05477b68414f3a Mon Sep 17 00:00:00 2001 From: Roo Code Date: Mon, 16 Feb 2026 20:32:42 +0000 Subject: [PATCH 3/4] fix: add TaskHistoryStore to task-persistence mock in Task.persistence.spec.ts The test mocks task-persistence with an explicit factory that was missing the new TaskHistoryStore export, causing all 9 tests to fail with "No TaskHistoryStore export is defined on the mock". --- src/core/task/__tests__/Task.persistence.spec.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/core/task/__tests__/Task.persistence.spec.ts b/src/core/task/__tests__/Task.persistence.spec.ts index 1e4acc9713b..e73638d8ad3 100644 --- a/src/core/task/__tests__/Task.persistence.spec.ts +++ b/src/core/task/__tests__/Task.persistence.spec.ts @@ -79,6 +79,17 @@ vi.mock("../../task-persistence", () => ({ readApiMessages: mockReadApiMessages, readTaskMessages: mockReadTaskMessages, taskMetadata: mockTaskMetadata, + TaskHistoryStore: vi.fn().mockImplementation(() => ({ + initialize: vi.fn().mockResolvedValue(undefined), + dispose: vi.fn(), + get: vi.fn(), + getAll: vi.fn().mockReturnValue([]), + upsert: vi.fn().mockResolvedValue([]), + delete: vi.fn().mockResolvedValue(undefined), + deleteMany: vi.fn().mockResolvedValue(undefined), + reconcile: vi.fn().mockResolvedValue(undefined), + initialized: Promise.resolve(), + })), })) vi.mock("vscode", () => { From 8941c612f6b952418fae52c916135959ce1e2bd8 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Mon, 16 Feb 2026 20:48:02 +0000 Subject: [PATCH 4/4] perf: debounce globalState write-through to avoid full-array writes on every mutation Instead of writing the entire HistoryItem[] array to globalState on every upsert/delete (expensive with 5000+ tasks), the write-through is now debounced with a 5-second window. Per-task file writes remain immediate (~200 bytes each). The globalState is flushed on dispose to ensure no data loss on shutdown. This makes the hot path during streaming (token count updates) write only the per-task file, not the full array. --- src/core/webview/ClineProvider.ts | 50 +++++++++++++++++-- .../ClineProvider.taskHistory.spec.ts | 38 +++++++------- 2 files changed, 65 insertions(+), 23 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 8281da85e7c..cd22afb6012 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -152,6 +152,8 @@ 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 private pendingOperations: Map = new Map() private static readonly PENDING_OPERATION_TIMEOUT_MS = 30000 // 30 seconds @@ -186,11 +188,12 @@ export class ClineProvider this.mdmService = mdmService this.updateGlobalState("codebaseIndexModels", EMBEDDING_MODEL_PROFILES) - // Initialize the per-task file-based history store with write-through callback. + // 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 (items) => { - // Write-through to globalState inside the store's lock for transition period - await this.updateGlobalState("taskHistory", items) + onWrite: async () => { + this.scheduleGlobalStateWriteThrough() }, }) this.initializeTaskHistoryStore().catch((error) => { @@ -709,6 +712,7 @@ export class ClineProvider this.marketplaceManager?.cleanup() this.customModesManager?.dispose() this.taskHistoryStore.dispose() + this.flushGlobalStateWriteThrough() this.log("Disposed all disposables") ClineProvider.activeInstances.delete(this) @@ -2613,6 +2617,44 @@ 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.taskHistory.spec.ts b/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts index 7fac36f59e6..b1f29008c4c 100644 --- a/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts @@ -508,18 +508,15 @@ describe("ClineProvider Task History Synchronization", () => { await provider.updateTaskHistory(updatedItem) - // Verify the update was persisted - expect(mockContext.globalState.update).toHaveBeenCalledWith( - "taskHistory", + // Verify the update was persisted in the store + const storeHistory = provider.taskHistoryStore.getAll() + expect(storeHistory).toEqual( expect.arrayContaining([expect.objectContaining({ id: "task-update", task: "Updated task" })]), ) // Should not have duplicates - const allCalls = (mockContext.globalState.update as ReturnType).mock.calls - const lastUpdateCall = allCalls.find((call: any[]) => call[0] === "taskHistory") - const historyArray = lastUpdateCall?.[1] as HistoryItem[] - const matchingItems = historyArray?.filter((item: HistoryItem) => item.id === "task-update") - expect(matchingItems?.length).toBe(1) + const matchingItems = storeHistory.filter((item: HistoryItem) => item.id === "task-update") + expect(matchingItems.length).toBe(1) }) it("returns the updated task history array", async () => { @@ -676,8 +673,8 @@ describe("ClineProvider Task History Synchronization", () => { await Promise.all(items.map((item) => provider.updateTaskHistory(item, { broadcast: false }))) - // All 5 entries must survive - const history = (provider as any).contextProxy.getGlobalState("taskHistory") as HistoryItem[] + // All 5 entries must survive (read from store, not debounced globalState) + const history = provider.taskHistoryStore.getAll() const ids = history.map((h: HistoryItem) => h.id) for (const item of items) { expect(ids).toContain(item.id) @@ -701,33 +698,36 @@ describe("ClineProvider Task History Synchronization", () => { provider.deleteTaskFromState("remove-me"), ]) - const history = (provider as any).contextProxy.getGlobalState("taskHistory") as HistoryItem[] + const history = provider.taskHistoryStore.getAll() const ids = history.map((h: HistoryItem) => h.id) expect(ids).toContain("keep-me") expect(ids).toContain("new-item") expect(ids).not.toContain("remove-me") }) - it("does not block subsequent writes when a previous write errors", async () => { + it("does not block subsequent writes when a previous store write errors", async () => { await provider.resolveWebviewView(mockWebviewView) - // Temporarily make the globalState write-through throw - const origUpdateGlobalState = (provider as any).updateGlobalState.bind(provider) + // Temporarily make the store's safeWriteJson throw + const { safeWriteJson } = await import("../../../utils/safeWriteJson") + const mockSafeWriteJson = vi.mocked(safeWriteJson) let callCount = 0 - ;(provider as any).updateGlobalState = vi.fn().mockImplementation((...args: unknown[]) => { + mockSafeWriteJson.mockImplementation(async () => { callCount++ if (callCount === 1) { - return Promise.reject(new Error("simulated write failure")) + throw new Error("simulated write failure") } - return origUpdateGlobalState(...args) }) - // First call should fail (due to write-through to globalState) + // First call should fail (store write failure) const item1 = createHistoryItem({ id: "fail-item", task: "Fail" }) await expect(provider.updateTaskHistory(item1, { broadcast: false })).rejects.toThrow( "simulated write failure", ) + // Restore mock + mockSafeWriteJson.mockResolvedValue(undefined) + // Second call should still succeed (store lock not stuck) const item2 = createHistoryItem({ id: "ok-item", task: "OK" }) const result = await provider.updateTaskHistory(item2, { broadcast: false }) @@ -750,7 +750,7 @@ describe("ClineProvider Task History Synchronization", () => { }), ]) - const history = (provider as any).contextProxy.getGlobalState("taskHistory") as HistoryItem[] + const history = provider.taskHistoryStore.getAll() const item = history.find((h: HistoryItem) => h.id === "race-item") expect(item).toBeDefined() // The second write (tokensIn: 222) should be the last one since writes are serialized