Revert "fix: use safeWriteJson for all JSON file writes" (RooCodeInc#4471)

Revert "fix: use safeWriteJson for all JSON file writes (RooCodeInc#3772)"

This reverts commit 1be30fc.

Author: mrubens

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

@@ -2,7 +2,6 @@ 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"
@@ -20,7 +19,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 safeWriteJson(path.join(cacheDir, filename), data)
+	await fs.writeFile(path.join(cacheDir, filename), JSON.stringify(data))
 }
 
 async function readModels(router: RouterName): Promise<ModelRecord | undefined> {

pnpm-lock.yaml

@@ -2,7 +2,6 @@ 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"
@@ -19,7 +18,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 safeWriteJson(path.join(cacheDir, filename), data)
+	await fs.writeFile(path.join(cacheDir, filename), JSON.stringify(data, null, 2))
 }
 
 async function readModelEndpoints(key: string): Promise<ModelRecord | undefined> {

src/api/providers/fetchers/modelCache.ts

@@ -12,7 +12,6 @@ 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: {
@@ -34,8 +33,6 @@ jest.mock("os", () => ({
 	homedir: jest.fn(() => "/mock/home"),
 }))
 
-jest.mock("../../../utils/safeWriteJson")
-
 describe("importExport", () => {
 	let mockProviderSettingsManager: jest.Mocked<ProviderSettingsManager>
 	let mockContextProxy: jest.Mocked<ContextProxy>
@@ -375,10 +372,11 @@ describe("importExport", () => {
 			expect(mockContextProxy.export).toHaveBeenCalled()
 			expect(fs.mkdir).toHaveBeenCalledWith("/mock/path", { recursive: true })
 
-			expect(safeWriteJson).toHaveBeenCalledWith("/mock/path/roo-code-settings.json", {
-				providerProfiles: mockProviderProfiles,
-				globalSettings: mockGlobalSettings,
-			})
+			expect(fs.writeFile).toHaveBeenCalledWith(
+				"/mock/path/roo-code-settings.json",
+				JSON.stringify({ providerProfiles: mockProviderProfiles, globalSettings: mockGlobalSettings }, null, 2),
+				"utf-8",
+			)
 		})
 
 		it("should include globalSettings when allowedMaxRequests is null", async () => {
@@ -407,10 +405,11 @@ describe("importExport", () => {
 				contextProxy: mockContextProxy,
 			})
 
-			expect(safeWriteJson).toHaveBeenCalledWith("/mock/path/roo-code-settings.json", {
-				providerProfiles: mockProviderProfiles,
-				globalSettings: mockGlobalSettings,
-			})
+			expect(fs.writeFile).toHaveBeenCalledWith(
+				"/mock/path/roo-code-settings.json",
+				JSON.stringify({ providerProfiles: mockProviderProfiles, globalSettings: mockGlobalSettings }, null, 2),
+				"utf-8",
+			)
 		})
 
 		it("should handle errors during the export process", async () => {
@@ -425,8 +424,7 @@ describe("importExport", () => {
 			})
 
 			mockContextProxy.export.mockResolvedValue({ mode: "code" })
-			// Simulate an error during the safeWriteJson operation
-			;(safeWriteJson as jest.Mock).mockRejectedValueOnce(new Error("Safe write error"))
+			;(fs.writeFile as jest.Mock).mockRejectedValue(new Error("Write error"))
 
 			await exportSettings({
 				providerSettingsManager: mockProviderSettingsManager,
@@ -437,10 +435,8 @@ describe("importExport", () => {
 			expect(mockProviderSettingsManager.export).toHaveBeenCalled()
 			expect(mockContextProxy.export).toHaveBeenCalled()
 			expect(fs.mkdir).toHaveBeenCalledWith("/mock/path", { recursive: true })
-			expect(safeWriteJson).toHaveBeenCalled() // safeWriteJson is called, but it will throw
+			expect(fs.writeFile).toHaveBeenCalled()
 			// 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/api/providers/fetchers/modelEndpointCache.ts

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

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

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

src/core/config/importExport.ts

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

src/core/context-tracking/FileContextTracker.ts

@@ -1,4 +1,3 @@
-import { safeWriteJson } from "../../utils/safeWriteJson"
 import * as path from "path"
 import * as fs from "fs/promises"
 
@@ -38,5 +37,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 safeWriteJson(filePath, messages)
+	await fs.writeFile(filePath, JSON.stringify(messages))
 }

src/core/task-persistence/apiMessages.ts

@@ -13,7 +13,6 @@ 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"
 
@@ -42,8 +41,6 @@ jest.mock("axios", () => ({
 	post: jest.fn(),
 }))
 
-jest.mock("../../../utils/safeWriteJson")
-
 jest.mock(
 	"@modelcontextprotocol/sdk/types.js",
 	() => ({
@@ -2004,7 +2001,10 @@ describe("Project MCP Settings", () => {
 		)
 
 		// Verify file was created with default content
-		expect(safeWriteJson).toHaveBeenCalledWith(expect.stringContaining("mcp.json"), { mcpServers: {} })
+		expect(fs.writeFile).toHaveBeenCalledWith(
+			expect.stringContaining("mcp.json"),
+			JSON.stringify({ mcpServers: {} }, null, 2),
+		)
 	})
 
 	test("handles openProjectMcpSettings when workspace is not open", async () => {