fix: use safeWriteJson for all JSON file writes

.roo/rules-code/use-safeWriteJson.md

@@ -0,0 +1,5 @@
+# JSON File Writing Must Be Atomic
+
+- MUST ALWAYS use `safeWriteJson(filePath: string, data: any): Promise<void>` from `src/utils/safeWriteJson.ts` instead of `JSON.stringify` with file-write operations
+- `safeWriteJson` prevents data corruption via atomic writes with locking and streams the write to minimize memory footprint
+- Test files are exempt from this rule

src/api/providers/fetchers/modelCache.ts

@@ -2,6 +2,7 @@ import * as path from "path"
 import fs from "fs/promises"
 
 import NodeCache from "node-cache"
+import { safeWriteJson } from "../../../utils/safeWriteJson"
 
 import { ContextProxy } from "../../../core/config/ContextProxy"
 import { getCacheDirectoryPath } from "../../../utils/storage"
@@ -19,7 +20,7 @@ const memoryCache = new NodeCache({ stdTTL: 5 * 60, checkperiod: 5 * 60 })
 async function writeModels(router: RouterName, data: ModelRecord) {
 	const filename = `${router}_models.json`
 	const cacheDir = await getCacheDirectoryPath(ContextProxy.instance.globalStorageUri.fsPath)
-	await fs.writeFile(path.join(cacheDir, filename), JSON.stringify(data))
+	await safeWriteJson(path.join(cacheDir, filename), data)
 }
 
 async function readModels(router: RouterName): Promise<ModelRecord | undefined> {

src/api/providers/fetchers/modelEndpointCache.ts

@@ -2,6 +2,7 @@ import * as path from "path"
 import fs from "fs/promises"
 
 import NodeCache from "node-cache"
+import { safeWriteJson } from "../../../utils/safeWriteJson"
 import sanitize from "sanitize-filename"
 
 import { ContextProxy } from "../../../core/config/ContextProxy"
@@ -18,7 +19,7 @@ const getCacheKey = (router: RouterName, modelId: string) => sanitize(`${router}
 async function writeModelEndpoints(key: string, data: ModelRecord) {
 	const filename = `${key}_endpoints.json`
 	const cacheDir = await getCacheDirectoryPath(ContextProxy.instance.globalStorageUri.fsPath)
-	await fs.writeFile(path.join(cacheDir, filename), JSON.stringify(data, null, 2))
+	await safeWriteJson(path.join(cacheDir, filename), data)
 }
 
 async function readModelEndpoints(key: string): Promise<ModelRecord | undefined> {

src/core/config/__tests__/importExport.test.ts

@@ -12,6 +12,7 @@ import { importSettings, exportSettings } from "../importExport"
 import { ProviderSettingsManager } from "../ProviderSettingsManager"
 import { ContextProxy } from "../ContextProxy"
 import { CustomModesManager } from "../CustomModesManager"
+import { safeWriteJson } from "../../../utils/safeWriteJson"
 
 jest.mock("vscode", () => ({
 	window: {
@@ -33,6 +34,8 @@ jest.mock("os", () => ({
 	homedir: jest.fn(() => "/mock/home"),
 }))
 
+jest.mock("../../../utils/safeWriteJson")
+
 describe("importExport", () => {
 	let mockProviderSettingsManager: jest.Mocked<ProviderSettingsManager>
 	let mockContextProxy: jest.Mocked<ContextProxy>
@@ -372,11 +375,10 @@ describe("importExport", () => {
 			expect(mockContextProxy.export).toHaveBeenCalled()
 			expect(fs.mkdir).toHaveBeenCalledWith("/mock/path", { recursive: true })
 
-			expect(fs.writeFile).toHaveBeenCalledWith(
-				"/mock/path/roo-code-settings.json",
-				JSON.stringify({ providerProfiles: mockProviderProfiles, globalSettings: mockGlobalSettings }, null, 2),
-				"utf-8",
-			)
+			expect(safeWriteJson).toHaveBeenCalledWith("/mock/path/roo-code-settings.json", {
+				providerProfiles: mockProviderProfiles,
+				globalSettings: mockGlobalSettings,
+			})
 		})
 
 		it("should include globalSettings when allowedMaxRequests is null", async () => {
@@ -405,11 +407,10 @@ describe("importExport", () => {
 				contextProxy: mockContextProxy,
 			})
 
-			expect(fs.writeFile).toHaveBeenCalledWith(
-				"/mock/path/roo-code-settings.json",
-				JSON.stringify({ providerProfiles: mockProviderProfiles, globalSettings: mockGlobalSettings }, null, 2),
-				"utf-8",
-			)
+			expect(safeWriteJson).toHaveBeenCalledWith("/mock/path/roo-code-settings.json", {
+				providerProfiles: mockProviderProfiles,
+				globalSettings: mockGlobalSettings,
+			})
 		})
 
 		it("should handle errors during the export process", async () => {
@@ -424,7 +425,8 @@ describe("importExport", () => {
 			})
 
 			mockContextProxy.export.mockResolvedValue({ mode: "code" })
-			;(fs.writeFile as jest.Mock).mockRejectedValue(new Error("Write error"))
+			// Simulate an error during the safeWriteJson operation
+			;(safeWriteJson as jest.Mock).mockRejectedValueOnce(new Error("Safe write error"))
 
 			await exportSettings({
 				providerSettingsManager: mockProviderSettingsManager,
@@ -435,8 +437,10 @@ describe("importExport", () => {
 			expect(mockProviderSettingsManager.export).toHaveBeenCalled()
 			expect(mockContextProxy.export).toHaveBeenCalled()
 			expect(fs.mkdir).toHaveBeenCalledWith("/mock/path", { recursive: true })
-			expect(fs.writeFile).toHaveBeenCalled()
+			expect(safeWriteJson).toHaveBeenCalled() // safeWriteJson is called, but it will throw
 			// The error is caught and the function exits silently.
+			// Optionally, ensure no error message was shown if that's part of "silent"
+			// expect(vscode.window.showErrorMessage).not.toHaveBeenCalled();
 		})
 
 		it("should handle errors during directory creation", async () => {

src/core/config/importExport.ts

@@ -1,3 +1,4 @@
+import { safeWriteJson } from "../../utils/safeWriteJson"
 import os from "os"
 import * as path from "path"
 import fs from "fs/promises"
@@ -116,6 +117,6 @@ export const exportSettings = async ({ providerSettingsManager, contextProxy }:
 
 		const dirname = path.dirname(uri.fsPath)
 		await fs.mkdir(dirname, { recursive: true })
-		await fs.writeFile(uri.fsPath, JSON.stringify({ providerProfiles, globalSettings }, null, 2), "utf-8")
+		await safeWriteJson(uri.fsPath, { providerProfiles, globalSettings })
 	} catch (e) {}
 }

src/core/context-tracking/FileContextTracker.ts

@@ -1,3 +1,4 @@
+import { safeWriteJson } from "../../utils/safeWriteJson"
 import * as path from "path"
 import * as vscode from "vscode"
 import { getTaskDirectoryPath } from "../../utils/storage"
@@ -130,7 +131,7 @@ export class FileContextTracker {
 			const globalStoragePath = this.getContextProxy()!.globalStorageUri.fsPath
 			const taskDir = await getTaskDirectoryPath(globalStoragePath, taskId)
 			const filePath = path.join(taskDir, GlobalFileNames.taskMetadata)
-			await fs.writeFile(filePath, JSON.stringify(metadata, null, 2))
+			await safeWriteJson(filePath, metadata)
 		} catch (error) {
 			console.error("Failed to save task metadata:", error)
 		}

src/core/task-persistence/apiMessages.ts

@@ -1,3 +1,4 @@
+import { safeWriteJson } from "../../utils/safeWriteJson"
 import * as path from "path"
 import * as fs from "fs/promises"
 
@@ -46,5 +47,5 @@ export async function saveApiMessages({
 }) {
 	const taskDir = await getTaskDirectoryPath(globalStoragePath, taskId)
 	const filePath = path.join(taskDir, GlobalFileNames.apiConversationHistory)
-	await fs.writeFile(filePath, JSON.stringify(messages))
+	await safeWriteJson(filePath, messages)
 }

src/core/task-persistence/taskMessages.ts

@@ -1,3 +1,4 @@
+import { safeWriteJson } from "../../utils/safeWriteJson"
 import * as path from "path"
 import * as fs from "fs/promises"
 
@@ -37,5 +38,5 @@ export type SaveTaskMessagesOptions = {
 export async function saveTaskMessages({ messages, taskId, globalStoragePath }: SaveTaskMessagesOptions) {
 	const taskDir = await getTaskDirectoryPath(globalStoragePath, taskId)
 	const filePath = path.join(taskDir, GlobalFileNames.uiMessages)
-	await fs.writeFile(filePath, JSON.stringify(messages))
+	await safeWriteJson(filePath, messages)
 }

src/core/webview/__tests__/ClineProvider.test.ts

@@ -13,6 +13,7 @@ import { experimentDefault } from "../../../shared/experiments"
 import { setTtsEnabled } from "../../../utils/tts"
 import { ContextProxy } from "../../config/ContextProxy"
 import { Task, TaskOptions } from "../../task/Task"
+import { safeWriteJson } from "../../../utils/safeWriteJson"
 
 import { ClineProvider } from "../ClineProvider"
 
@@ -41,6 +42,8 @@ jest.mock("axios", () => ({
 	post: jest.fn(),
 }))
 
+jest.mock("../../../utils/safeWriteJson")
+
 jest.mock(
 	"@modelcontextprotocol/sdk/types.js",
 	() => ({
@@ -2000,10 +2003,7 @@ describe("Project MCP Settings", () => {
 		)
 
 		// Verify file was created with default content
-		expect(fs.writeFile).toHaveBeenCalledWith(
-			expect.stringContaining("mcp.json"),
-			JSON.stringify({ mcpServers: {} }, null, 2),
-		)
+		expect(safeWriteJson).toHaveBeenCalledWith(expect.stringContaining("mcp.json"), { mcpServers: {} })
 	})
 
 	test("handles openProjectMcpSettings when workspace is not open", async () => {

src/core/webview/webviewMessageHandler.ts

@@ -1,3 +1,4 @@
+import { safeWriteJson } from "../../utils/safeWriteJson"
 import * as path from "path"
 import fs from "fs/promises"
 import pWaitFor from "p-wait-for"
@@ -478,7 +479,7 @@ export const webviewMessageHandler = async (provider: ClineProvider, message: We
 				const exists = await fileExistsAtPath(mcpPath)
 
 				if (!exists) {
-					await fs.writeFile(mcpPath, JSON.stringify({ mcpServers: {} }, null, 2))
+					await safeWriteJson(mcpPath, { mcpServers: {} })
 				}
 
 				await openFile(mcpPath)