diff --git a/src/core/task-persistence/TaskHistoryStore.ts b/src/core/task-persistence/TaskHistoryStore.ts new file mode 100644 index 00000000000..4157d8b9fbb --- /dev/null +++ b/src/core/task-persistence/TaskHistoryStore.ts @@ -0,0 +1,572 @@ +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. + */ +/** + * 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 + private fsWatcher: fsSync.FSWatcher | null = null + 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, options?: TaskHistoryStoreOptions) { + this.globalStoragePath = globalStoragePath + this.onWrite = options?.onWrite + this.initialized = new Promise((resolve) => { + this.resolveInitialized = resolve + }) + } + + // ────────────────────────────── Lifecycle ────────────────────────────── + + /** + * Load index, reconcile if needed, start watchers. + */ + async initialize(): Promise { + try { + 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() + } finally { + // Mark initialization as complete so callers awaiting `initialized` can proceed + this.resolveInitialized() + } + } + + /** + * 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() + + const all = this.getAll() + + // Call onWrite callback inside the lock for serialized write-through + if (this.onWrite) { + await this.onWrite(all) + } + + return all + }) + } + + /** + * 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() + + // Call onWrite callback inside the lock for serialized write-through + if (this.onWrite) { + await this.onWrite(this.getAll()) + } + }) + } + + /** + * 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() + + // Call onWrite callback inside the lock for serialized write-through + if (this.onWrite) { + await this.onWrite(this.getAll()) + } + }) + } + + // ────────────────────────────── 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 { + // Run through the write lock to prevent interleaving with upsert/delete + return this.withLock(async () => { + 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/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", () => { diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index bb9199a65c2..cd22afb6012 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,10 @@ export class ClineProvider private _disposed = false private recentTasksCache?: string[] - private taskHistoryWriteLock: Promise = Promise.resolve() + 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 @@ -185,6 +188,18 @@ export class ClineProvider this.mdmService = mdmService 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() + }, + }) + 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 +329,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 +711,8 @@ export class ClineProvider this.skillsManager = undefined this.marketplaceManager?.cleanup() this.customModesManager?.dispose() + this.taskHistoryStore.dispose() + this.flushGlobalStateWriteThrough() this.log("Disposed all disposables") ClineProvider.activeInstances.delete(this) @@ -1344,12 +1390,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 +1609,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 +1770,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 +1903,8 @@ 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 // Delete associated shadow repositories or branches and task directories const globalStorageDir = this.contextProxy.globalStorageUri.fsPath @@ -1902,12 +1945,9 @@ 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 + await this.postStateToWebview() } @@ -2074,6 +2114,9 @@ export class ClineProvider } async getStateToPostToWebview(): Promise { + // Ensure the store is initialized before reading task history + await this.taskHistoryStore.initialized + const { apiConfiguration, lastShownAnnouncementId, @@ -2206,14 +2249,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 +2484,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 +2593,79 @@ 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 = this.taskHistoryStore.get(item.id) ?? item + await this.postMessageToWebview({ type: "taskHistoryItemUpdated", taskHistoryItem: updatedItem }) + } + + 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 = wasExisting ? history[existingItemIndex] : item - await this.postMessageToWebview({ type: "taskHistoryItemUpdated", taskHistoryItem: updatedItem }) + /** + * 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 + } - return history + 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. - * @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 +2916,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..b1f29008c4c 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), @@ -496,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 () => { @@ -582,18 +591,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 +610,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 +620,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 +639,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 +649,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() @@ -665,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) @@ -690,34 +698,37 @@ 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 updateGlobalState 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 + // 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", ) - // Second call should still succeed (lock not stuck) + // 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 }) expect(result.some((h) => h.id === "ok-item")).toBe(true) @@ -739,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 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", }