3 of 3: scalability fix: refactor task history persistence to use file-based storage

.roo/rules/use-safeReadJson.md

@@ -0,0 +1,33 @@
+# JSON File Reading Must Be Safe and Atomic
+
+- You MUST use `safeReadJson(filePath: string, jsonPath?: string | string[]): Promise<any>` from `src/utils/safeReadJson.ts` to read JSON files
+- `safeReadJson` provides atomic file access to local files with proper locking to prevent race conditions and uses `stream-json` to read JSON files without buffering to a string
+- Test files are exempt from this rule
+
+## Correct Usage Example
+
+This pattern replaces all manual `fs` or `vscode.workspace.fs` reads.
+
+### ❌ Don't do this:
+
+```typescript
+// Anti-patterns: string buffering wastes memory
+const data = JSON.parse(await fs.readFile(filePath, 'utf8'));
+const data = JSON.parse(await vscode.workspace.fs.readFile(fileUri));
+
+// Anti-pattern: Unsafe existence check
+if (await fileExists.. ) { /* then read */ }
+```
+
+### ✅ Use this unified pattern:
+
+```typescript
+let data
+try {
+	data = await safeReadJson(filePath)
+} catch (error) {
+	if (error.code !== "ENOENT") {
+		// Handle at least ENOENT
+	}
+}
+```

.roo/rules/use-safeWriteJson.md

@@ -0,0 +1,11 @@
+# JSON File Writing Must Be Atomic
+
+- You MUST use `safeWriteJson(filePath: string, data: any): Promise<void>` from `src/utils/safeWriteJson.ts` instead of `JSON.stringify` with file-write operations
+- `safeWriteJson` will create parent directories if necessary, so do not call `mkdir` prior to `safeWriteJson`
+- `safeWriteJson` prevents data corruption via atomic writes with locking and streams the write to minimize memory footprint
+- Use the `readModifyFn` parameter of `safeWriteJson` to perform atomic transactions: `safeWriteJson(filePath, requiredDefaultValue, async (data) => { /* modify `data`in place and return`data` to save changes, or return undefined to cancel the operation without writing */ })`
+    - When using readModifyFn with default data, it must be a modifiable type (object or array)
+    - for memory efficiency, `data` must be modified in-place: prioritize the use of push/pop/splice/truncate and maintain the original reference
+    - if and only if the operation being performed on `data` is impossible without new reference creation may it return a reference other than `data`
+        - you must assign any new references to structures needed outside of the critical section from within readModifyFn before returning: you must avoid `obj = await safeWriteJson()` which could introduce race conditions from the non-deterministic execution ordering of await
+- Test files are exempt from these rules

packages/types/src/global-settings.ts

@@ -7,7 +7,6 @@ import {
 	providerSettingsEntrySchema,
 	providerSettingsSchema,
 } from "./provider-settings.js"
-import { historyItemSchema } from "./history.js"
 import { codebaseIndexModelsSchema, codebaseIndexConfigSchema } from "./codebase-index.js"
 import { experimentsSchema } from "./experiment.js"
 import { telemetrySettingsSchema } from "./telemetry.js"
@@ -26,8 +25,6 @@ export const globalSettingsSchema = z.object({
 
 	lastShownAnnouncementId: z.string().optional(),
 	customInstructions: z.string().optional(),
-	taskHistory: z.array(historyItemSchema).optional(),
-
 	condensingApiConfigId: z.string().optional(),
 	customCondensingPrompt: z.string().optional(),
 

packages/types/src/history.ts

@@ -19,3 +19,56 @@ export const historyItemSchema = z.object({
 })
 
 export type HistoryItem = z.infer<typeof historyItemSchema>
+
+/**
+ * HistorySearchResultItem - extends HistoryItem with match positions from fzf
+ */
+export const historySearchResultItemSchema = historyItemSchema.extend({
+	match: z
+		.object({
+			positions: z.array(z.number()),
+		})
+		.optional(),
+})
+
+export type HistorySearchResultItem = z.infer<typeof historySearchResultItemSchema>
+
+/**
+ * HistorySearchResults - contains a list of search results with match information
+ * and unique workspaces encountered during the search
+ */
+/**
+ * HistoryWorkspaceItem - represents a workspace with metadata
+ */
+export const historyWorkspaceItemSchema = z.object({
+	path: z.string(),
+	name: z.string(),
+	missing: z.boolean(),
+	ts: z.number(),
+})
+
+export type HistoryWorkspaceItem = z.infer<typeof historyWorkspaceItemSchema>
+
+export const historySearchResultsSchema = z.object({
+	items: z.array(historySearchResultItemSchema),
+	workspaces: z.array(z.string()).optional(),
+	workspaceItems: z.array(historyWorkspaceItemSchema).optional(),
+})
+
+export type HistorySearchResults = z.infer<typeof historySearchResultsSchema>
+
+/**
+ * Sort options for history items
+ */
+export type HistorySortOption = "newest" | "oldest" | "mostExpensive" | "mostTokens" | "mostRelevant"
+
+/**
+ * HistorySearchOptions
+ */
+export interface HistorySearchOptions {
+	searchQuery?: string
+	limit?: number
+	workspacePath?: string
+	sortOption?: HistorySortOption
+	dateRange?: { fromTs?: number; toTs?: number }
+}

src/api/providers/fetchers/modelCache.ts

@@ -2,12 +2,12 @@ import * as path from "path"
 import fs from "fs/promises"
 
 import NodeCache from "node-cache"
+import { safeReadJson } from "../../../utils/safeReadJson"
 import { safeWriteJson } from "../../../utils/safeWriteJson"
 
 import { ContextProxy } from "../../../core/config/ContextProxy"
 import { getCacheDirectoryPath } from "../../../utils/storage"
 import { RouterName, ModelRecord } from "../../../shared/api"
-import { fileExistsAtPath } from "../../../utils/fs"
 
 import { getOpenRouterModels } from "./openrouter"
 import { getRequestyModels } from "./requesty"
@@ -30,8 +30,14 @@ async function readModels(router: RouterName): Promise<ModelRecord | undefined>
 	const filename = `${router}_models.json`
 	const cacheDir = await getCacheDirectoryPath(ContextProxy.instance.globalStorageUri.fsPath)
 	const filePath = path.join(cacheDir, filename)
-	const exists = await fileExistsAtPath(filePath)
-	return exists ? JSON.parse(await fs.readFile(filePath, "utf8")) : undefined
+	try {
+		return await safeReadJson(filePath)
+	} catch (error: any) {
+		if (error.code === "ENOENT") {
+			return undefined
+		}
+		throw error
+	}
 }
 
 /**

src/api/providers/fetchers/modelEndpointCache.ts

@@ -2,13 +2,13 @@ import * as path from "path"
 import fs from "fs/promises"
 
 import NodeCache from "node-cache"
+import { safeReadJson } from "../../../utils/safeReadJson"
 import { safeWriteJson } from "../../../utils/safeWriteJson"
 import sanitize from "sanitize-filename"
 
 import { ContextProxy } from "../../../core/config/ContextProxy"
 import { getCacheDirectoryPath } from "../../../utils/storage"
 import { RouterName, ModelRecord } from "../../../shared/api"
-import { fileExistsAtPath } from "../../../utils/fs"
 
 import { getOpenRouterModelEndpoints } from "./openrouter"
 
@@ -26,8 +26,11 @@ async function readModelEndpoints(key: string): Promise<ModelRecord | undefined>
 	const filename = `${key}_endpoints.json`
 	const cacheDir = await getCacheDirectoryPath(ContextProxy.instance.globalStorageUri.fsPath)
 	const filePath = path.join(cacheDir, filename)
-	const exists = await fileExistsAtPath(filePath)
-	return exists ? JSON.parse(await fs.readFile(filePath, "utf8")) : undefined
+	try {
+		return await safeReadJson(filePath)
+	} catch (error) {
+		return undefined
+	}
 }
 
 export const getModelEndpoints = async ({

src/core/checkpoints/index.ts

@@ -199,15 +199,19 @@ export async function checkpointRestore(cline: Task, { ts, commitHash, mode }: C
 		await provider?.postMessageToWebview({ type: "currentCheckpointUpdated", text: commitHash })
 
 		if (mode === "restore") {
-			await cline.overwriteApiConversationHistory(cline.apiConversationHistory.filter((m) => !m.ts || m.ts < ts))
+			await cline.modifyApiConversationHistory(async (history) => {
+				return history.filter((m) => !m.ts || m.ts < ts)
+			})
 
 			const deletedMessages = cline.clineMessages.slice(index + 1)
 
 			const { totalTokensIn, totalTokensOut, totalCacheWrites, totalCacheReads, totalCost } = getApiMetrics(
 				cline.combineMessages(deletedMessages),
 			)
 
-			await cline.overwriteClineMessages(cline.clineMessages.slice(0, index + 1))
+			await cline.modifyClineMessages(async (messages) => {
+				return messages.slice(0, index + 1)
+			})
 
 			// TODO: Verify that this is working as expected.
 			await cline.say(

src/core/config/ContextProxy.ts

@@ -23,12 +23,11 @@ type GlobalStateKey = keyof GlobalState
 type SecretStateKey = keyof SecretState
 type RooCodeSettingsKey = keyof RooCodeSettings
 
-const PASS_THROUGH_STATE_KEYS = ["taskHistory"]
+const PASS_THROUGH_STATE_KEYS: string[] = []
 
 export const isPassThroughStateKey = (key: string) => PASS_THROUGH_STATE_KEYS.includes(key)
 
 const globalSettingsExportSchema = globalSettingsSchema.omit({
-	taskHistory: true,
 	listApiConfigMeta: true,
 	currentApiConfigName: true,
 })

src/core/config/__tests__/ContextProxy.spec.ts

@@ -102,41 +102,6 @@ describe("ContextProxy", () => {
 			const result = proxy.getGlobalState("apiProvider", "deepseek")
 			expect(result).toBe("deepseek")
 		})
-
-		it("should bypass cache for pass-through state keys", async () => {
-			// Setup mock return value
-			mockGlobalState.get.mockReturnValue("pass-through-value")
-
-			// Use a pass-through key (taskHistory)
-			const result = proxy.getGlobalState("taskHistory")
-
-			// Should get value directly from original context
-			expect(result).toBe("pass-through-value")
-			expect(mockGlobalState.get).toHaveBeenCalledWith("taskHistory")
-		})
-
-		it("should respect default values for pass-through state keys", async () => {
-			// Setup mock to return undefined
-			mockGlobalState.get.mockReturnValue(undefined)
-
-			// Use a pass-through key with default value
-			const historyItems = [
-				{
-					id: "1",
-					number: 1,
-					ts: 1,
-					task: "test",
-					tokensIn: 1,
-					tokensOut: 1,
-					totalCost: 1,
-				},
-			]
-
-			const result = proxy.getGlobalState("taskHistory", historyItems)
-
-			// Should return default value when original context returns undefined
-			expect(result).toBe(historyItems)
-		})
 	})
 
 	describe("updateGlobalState", () => {
@@ -150,33 +115,6 @@ describe("ContextProxy", () => {
 			const storedValue = await proxy.getGlobalState("apiProvider")
 			expect(storedValue).toBe("deepseek")
 		})
-
-		it("should bypass cache for pass-through state keys", async () => {
-			const historyItems = [
-				{
-					id: "1",
-					number: 1,
-					ts: 1,
-					task: "test",
-					tokensIn: 1,
-					tokensOut: 1,
-					totalCost: 1,
-				},
-			]
-
-			await proxy.updateGlobalState("taskHistory", historyItems)
-
-			// Should update original context
-			expect(mockGlobalState.update).toHaveBeenCalledWith("taskHistory", historyItems)
-
-			// Setup mock for subsequent get
-			mockGlobalState.get.mockReturnValue(historyItems)
-
-			// Should get fresh value from original context
-			const storedValue = proxy.getGlobalState("taskHistory")
-			expect(storedValue).toBe(historyItems)
-			expect(mockGlobalState.get).toHaveBeenCalledWith("taskHistory")
-		})
 	})
 
 	describe("getSecret", () => {