refactor: replace fs.readFile + JSON.parse with safeReadJson

Replace manual file reading and JSON parsing with the safer safeReadJson utility
across multiple files in the codebase. This change:

- Provides atomic file access with proper locking to prevent race conditions
- Streams file contents efficiently for better memory usage
- Improves error handling consistency
- Reduces code duplication

Fixes: #5331
Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org>

src/api/providers/fetchers/modelCache.ts

@@ -2,15 +2,13 @@ import * as path from "path"
 import fs from "fs/promises"
 
 import NodeCache from "node-cache"
-
 import type { ProviderName } from "@roo-code/types"
-
+import { safeReadJson } from "../../../utils/safeReadJson"
 import { safeWriteJson } from "../../../utils/safeWriteJson"
 
 import { ContextProxy } from "../../../core/config/ContextProxy"
 import { getCacheDirectoryPath } from "../../../utils/storage"
 import type { RouterName, ModelRecord } from "../../../shared/api"
-import { fileExistsAtPath } from "../../../utils/fs"
 
 import { getOpenRouterModels } from "./openrouter"
 import { getVercelAiGatewayModels } from "./vercel-ai-gateway"
@@ -37,8 +35,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/config/importExport.ts

@@ -1,3 +1,4 @@
+import { safeReadJson } from "../../utils/safeReadJson"
 import { safeWriteJson } from "../../utils/safeWriteJson"
 import os from "os"
 import * as path from "path"
@@ -49,7 +50,7 @@ export async function importSettingsFromPath(
 		const previousProviderProfiles = await providerSettingsManager.export()
 
 		const { providerProfiles: newProviderProfiles, globalSettings = {} } = schema.parse(
-			JSON.parse(await fs.readFile(filePath, "utf-8")),
+			await safeReadJson(filePath),
 		)
 
 		const providerProfiles = {

src/core/context-tracking/FileContextTracker.ts

@@ -1,10 +1,9 @@
+import { safeReadJson } from "../../utils/safeReadJson"
 import { safeWriteJson } from "../../utils/safeWriteJson"
 import * as path from "path"
 import * as vscode from "vscode"
 import { getTaskDirectoryPath } from "../../utils/storage"
 import { GlobalFileNames } from "../../shared/globalFileNames"
-import { fileExistsAtPath } from "../../utils/fs"
-import fs from "fs/promises"
 import { ContextProxy } from "../config/ContextProxy"
 import type { FileMetadataEntry, RecordSource, TaskMetadata } from "./FileContextTrackerTypes"
 import { ClineProvider } from "../webview/ClineProvider"
@@ -116,12 +115,14 @@ export class FileContextTracker {
 		const taskDir = await getTaskDirectoryPath(globalStoragePath, taskId)
 		const filePath = path.join(taskDir, GlobalFileNames.taskMetadata)
 		try {
-			if (await fileExistsAtPath(filePath)) {
-				return JSON.parse(await fs.readFile(filePath, "utf8"))
-			}
+			return await safeReadJson(filePath)
 		} catch (error) {
-			console.error("Failed to read task metadata:", error)
+			if (error.code !== "ENOENT") {
+				console.error("Failed to read task metadata:", error)
+			}
 		}
+
+		// On error, return default empty metadata
 		return { files_in_context: [] }
 	}
 

src/core/task-persistence/apiMessages.ts

@@ -1,3 +1,4 @@
+import { safeReadJson } from "../../utils/safeReadJson"
 import { safeWriteJson } from "../../utils/safeWriteJson"
 import * as path from "path"
 import * as fs from "fs/promises"
@@ -21,29 +22,21 @@ export async function readApiMessages({
 	const taskDir = await getTaskDirectoryPath(globalStoragePath, taskId)
 	const filePath = path.join(taskDir, GlobalFileNames.apiConversationHistory)
 
-	if (await fileExistsAtPath(filePath)) {
-		const fileContent = await fs.readFile(filePath, "utf8")
-		try {
-			const parsedData = JSON.parse(fileContent)
-			if (Array.isArray(parsedData) && parsedData.length === 0) {
-				console.error(
-					`[Roo-Debug] readApiMessages: Found API conversation history file, but it's empty (parsed as []). TaskId: ${taskId}, Path: ${filePath}`,
-				)
-			}
-			return parsedData
-		} catch (error) {
+	try {
+		const parsedData = await safeReadJson(filePath)
+		if (Array.isArray(parsedData) && parsedData.length === 0) {
 			console.error(
-				`[Roo-Debug] readApiMessages: Error parsing API conversation history file. TaskId: ${taskId}, Path: ${filePath}, Error: ${error}`,
+				`[Roo-Debug] readApiMessages: Found API conversation history file, but it's empty (parsed as []). TaskId: ${taskId}, Path: ${filePath}`,
 			)
-			throw error
 		}
-	} else {
-		const oldPath = path.join(taskDir, "claude_messages.json")
+		return parsedData
+	} catch (error: any) {
+		if (error.code === "ENOENT") {
+			// File doesn't exist, try the old path
+			const oldPath = path.join(taskDir, "claude_messages.json")
 
-		if (await fileExistsAtPath(oldPath)) {
-			const fileContent = await fs.readFile(oldPath, "utf8")
 			try {
-				const parsedData = JSON.parse(fileContent)
+				const parsedData = await safeReadJson(oldPath)
 				if (Array.isArray(parsedData) && parsedData.length === 0) {
 					console.error(
 						`[Roo-Debug] readApiMessages: Found OLD API conversation history file (claude_messages.json), but it's empty (parsed as []). TaskId: ${taskId}, Path: ${oldPath}`,
@@ -51,21 +44,29 @@ export async function readApiMessages({
 				}
 				await fs.unlink(oldPath)
 				return parsedData
-			} catch (error) {
+			} catch (oldError: any) {
+				if (oldError.code === "ENOENT") {
+					// If we reach here, neither the new nor the old history file was found.
+					console.error(
+						`[Roo-Debug] readApiMessages: API conversation history file not found for taskId: ${taskId}. Expected at: ${filePath}`,
+					)
+					return []
+				}
+
+				// For any other error with the old file, log and rethrow
 				console.error(
-					`[Roo-Debug] readApiMessages: Error parsing OLD API conversation history file (claude_messages.json). TaskId: ${taskId}, Path: ${oldPath}, Error: ${error}`,
+					`[Roo-Debug] readApiMessages: Error reading OLD API conversation history file (claude_messages.json). TaskId: ${taskId}, Path: ${oldPath}, Error: ${oldError}`,
 				)
-				// DO NOT unlink oldPath if parsing failed, throw error instead.
-				throw error
+				throw oldError
 			}
+		} else {
+			// For any other error with the main file, log and rethrow
+			console.error(
+				`[Roo-Debug] readApiMessages: Error reading API conversation history file. TaskId: ${taskId}, Path: ${filePath}, Error: ${error}`,
+			)
+			throw error
 		}
 	}
-
-	// If we reach here, neither the new nor the old history file was found.
-	console.error(
-		`[Roo-Debug] readApiMessages: API conversation history file not found for taskId: ${taskId}. Expected at: ${filePath}`,
-	)
-	return []
 }
 
 export async function saveApiMessages({

src/core/task-persistence/taskMessages.ts

@@ -1,11 +1,9 @@
+import { safeReadJson } from "../../utils/safeReadJson"
 import { safeWriteJson } from "../../utils/safeWriteJson"
 import * as path from "path"
-import * as fs from "fs/promises"
 
 import type { ClineMessage } from "@roo-code/types"
 
-import { fileExistsAtPath } from "../../utils/fs"
-
 import { GlobalFileNames } from "../../shared/globalFileNames"
 import { getTaskDirectoryPath } from "../../utils/storage"
 
@@ -20,13 +18,15 @@ export async function readTaskMessages({
 }: ReadTaskMessagesOptions): Promise<ClineMessage[]> {
 	const taskDir = await getTaskDirectoryPath(globalStoragePath, taskId)
 	const filePath = path.join(taskDir, GlobalFileNames.uiMessages)
-	const fileExists = await fileExistsAtPath(filePath)
 
-	if (fileExists) {
-		return JSON.parse(await fs.readFile(filePath, "utf8"))
+	try {
+		return await safeReadJson(filePath)
+	} catch (error) {
+		if (error.code !== "ENOENT") {
+			console.error("Failed to read task messages:", error)
+		}
+		return []
 	}
-
-	return []
 }
 
 export type SaveTaskMessagesOptions = {

src/core/webview/ClineProvider.ts

@@ -82,6 +82,7 @@ import { t } from "../../i18n"
 import { buildApiHandler } from "../../api"
 import { forceFullModelDetailsLoad, hasLoadedFullDetails } from "../../api/providers/fetchers/lmstudio"
 
+import { safeReadJson } from "../../utils/safeReadJson"
 import { ContextProxy } from "../config/ContextProxy"
 import { ProviderSettingsManager } from "../config/ProviderSettingsManager"
 import { CustomModesManager } from "../config/CustomModesManager"
@@ -1457,10 +1458,9 @@ export class ClineProvider
 			const taskDirPath = await getTaskDirectoryPath(globalStoragePath, id)
 			const apiConversationHistoryFilePath = path.join(taskDirPath, GlobalFileNames.apiConversationHistory)
 			const uiMessagesFilePath = path.join(taskDirPath, GlobalFileNames.uiMessages)
-			const fileExists = await fileExistsAtPath(apiConversationHistoryFilePath)
 
-			if (fileExists) {
-				const apiConversationHistory = JSON.parse(await fs.readFile(apiConversationHistoryFilePath, "utf8"))
+			try {
+				const apiConversationHistory = await safeReadJson(apiConversationHistoryFilePath)
 
 				return {
 					historyItem,
@@ -1469,6 +1469,10 @@ export class ClineProvider
 					uiMessagesFilePath,
 					apiConversationHistory,
 				}
+			} catch (error) {
+				if (error.code !== "ENOENT") {
+					console.error(`Failed to read API conversation history for task ${id}:`, error)
+				}
 			}
 		}
 

src/integrations/misc/extract-text.ts

@@ -7,6 +7,7 @@ import { isBinaryFile } from "isbinaryfile"
 import { extractTextFromXLSX } from "./extract-text-from-xlsx"
 import { countFileLines } from "./line-counter"
 import { readLines } from "./read-lines"
+import { safeReadJson } from "../../utils/safeReadJson"
 
 async function extractTextFromPDF(filePath: string): Promise<string> {
 	const dataBuffer = await fs.readFile(filePath)
@@ -20,8 +21,7 @@ async function extractTextFromDOCX(filePath: string): Promise<string> {
 }
 
 async function extractTextFromIPYNB(filePath: string): Promise<string> {
-	const data = await fs.readFile(filePath, "utf8")
-	const notebook = JSON.parse(data)
+	const notebook = await safeReadJson(filePath)
 	let extractedText = ""
 
 	for (const cell of notebook.cells) {

src/services/code-index/cache-manager.ts

@@ -2,6 +2,7 @@ import * as vscode from "vscode"
 import { createHash } from "crypto"
 import { ICacheManager } from "./interfaces/cache"
 import debounce from "lodash.debounce"
+import { safeReadJson } from "../../utils/safeReadJson"
 import { safeWriteJson } from "../../utils/safeWriteJson"
 import { TelemetryService } from "@roo-code/telemetry"
 import { TelemetryEventName } from "@roo-code/types"
@@ -37,8 +38,7 @@ export class CacheManager implements ICacheManager {
 	 */
 	async initialize(): Promise<void> {
 		try {
-			const cacheData = await vscode.workspace.fs.readFile(this.cachePath)
-			this.fileHashes = JSON.parse(cacheData.toString())
+			this.fileHashes = await safeReadJson(this.cachePath.fsPath)
 		} catch (error) {
 			this.fileHashes = {}
 			TelemetryService.instance.captureEvent(TelemetryEventName.CODE_INDEX_ERROR, {

src/services/marketplace/MarketplaceManager.ts

@@ -15,6 +15,7 @@ import type { CustomModesManager } from "../../core/config/CustomModesManager"
 
 import { RemoteConfigLoader } from "./RemoteConfigLoader"
 import { SimpleInstaller } from "./SimpleInstaller"
+import { safeReadJson } from "../../utils/safeReadJson"
 
 export interface MarketplaceItemsResponse {
 	organizationMcps: MarketplaceItem[]
@@ -272,8 +273,7 @@ export class MarketplaceManager {
 			// Check MCPs in .roo/mcp.json
 			const projectMcpPath = path.join(workspaceFolder.uri.fsPath, ".roo", "mcp.json")
 			try {
-				const content = await fs.readFile(projectMcpPath, "utf-8")
-				const data = JSON.parse(content)
+				const data = await safeReadJson(projectMcpPath)
 				if (data?.mcpServers && typeof data.mcpServers === "object") {
 					for (const serverName of Object.keys(data.mcpServers)) {
 						metadata[serverName] = {
@@ -317,8 +317,7 @@ export class MarketplaceManager {
 			// Check global MCPs
 			const globalMcpPath = path.join(globalSettingsPath, GlobalFileNames.mcpSettings)
 			try {
-				const content = await fs.readFile(globalMcpPath, "utf-8")
-				const data = JSON.parse(content)
+				const data = await safeReadJson(globalMcpPath)
 				if (data?.mcpServers && typeof data.mcpServers === "object") {
 					for (const serverName of Object.keys(data.mcpServers)) {
 						metadata[serverName] = {

src/services/marketplace/SimpleInstaller.ts

@@ -6,6 +6,7 @@ import type { MarketplaceItem, MarketplaceItemType, InstallMarketplaceItemOption
 import { GlobalFileNames } from "../../shared/globalFileNames"
 import { ensureSettingsDirectoryExists } from "../../utils/globalContext"
 import type { CustomModesManager } from "../../core/config/CustomModesManager"
+import { safeReadJson } from "../../utils/safeReadJson"
 
 export interface InstallOptions extends InstallMarketplaceItemOptions {
 	target: "project" | "global"
@@ -229,8 +230,7 @@ export class SimpleInstaller {
 		// Read existing file or create new structure
 		let existingData: any = { mcpServers: {} }
 		try {
-			const existing = await fs.readFile(filePath, "utf-8")
-			existingData = JSON.parse(existing) || { mcpServers: {} }
+			existingData = (await safeReadJson(filePath)) || { mcpServers: {} }
 		} catch (error: any) {
 			if (error.code === "ENOENT") {
 				// File doesn't exist, use default structure
@@ -332,8 +332,7 @@ export class SimpleInstaller {
 		const filePath = await this.getMcpFilePath(target)
 
 		try {
-			const existing = await fs.readFile(filePath, "utf-8")
-			const existingData = JSON.parse(existing)
+			const existingData = await safeReadJson(filePath)
 
 			if (existingData?.mcpServers) {
 				// Parse the item content to get server names

src/services/mcp/McpHub.ts

@@ -18,6 +18,8 @@ import * as path from "path"
 import * as vscode from "vscode"
 import { z } from "zod"
 import { t } from "../../i18n"
+import { safeReadJson } from "../../utils/safeReadJson"
+import { safeWriteJson } from "../../utils/safeWriteJson"
 
 import { ClineProvider } from "../../core/webview/ClineProvider"
 import { GlobalFileNames } from "../../shared/globalFileNames"
@@ -297,11 +299,9 @@ export class McpHub {
 
 	private async handleConfigFileChange(filePath: string, source: "global" | "project"): Promise<void> {
 		try {
-			const content = await fs.readFile(filePath, "utf-8")
 			let config: any
-
 			try {
-				config = JSON.parse(content)
+				config = await safeReadJson(filePath)
 			} catch (parseError) {
 				const errorMessage = t("mcp:errors.invalid_settings_syntax")
 				console.error(errorMessage, parseError)
@@ -383,11 +383,9 @@ export class McpHub {
 			const projectMcpPath = await this.getProjectMcpPath()
 			if (!projectMcpPath) return
 
-			const content = await fs.readFile(projectMcpPath, "utf-8")
 			let config: any
-
 			try {
-				config = JSON.parse(content)
+				config = await safeReadJson(projectMcpPath)
 			} catch (parseError) {
 				const errorMessage = t("mcp:errors.invalid_settings_syntax")
 				console.error(errorMessage, parseError)
@@ -511,8 +509,7 @@ export class McpHub {
 				return
 			}
 
-			const content = await fs.readFile(configPath, "utf-8")
-			const config = JSON.parse(content)
+			const config = await safeReadJson(configPath)
 			const result = McpSettingsSchema.safeParse(config)
 
 			if (result.success) {
@@ -927,14 +924,12 @@ export class McpHub {
 					const projectMcpPath = await this.getProjectMcpPath()
 					if (projectMcpPath) {
 						configPath = projectMcpPath
-						const content = await fs.readFile(configPath, "utf-8")
-						serverConfigData = JSON.parse(content)
+						serverConfigData = await safeReadJson(configPath)
 					}
 				} else {
 					// Get global MCP settings path
 					configPath = await this.getMcpSettingsFilePath()
-					const content = await fs.readFile(configPath, "utf-8")
-					serverConfigData = JSON.parse(content)
+					serverConfigData = await safeReadJson(configPath)
 				}
 				if (serverConfigData) {
 					alwaysAllowConfig = serverConfigData.mcpServers?.[serverName]?.alwaysAllow || []
@@ -1236,8 +1231,7 @@ export class McpHub {
 			const globalPath = await this.getMcpSettingsFilePath()
 			let globalServers: Record<string, any> = {}
 			try {
-				const globalContent = await fs.readFile(globalPath, "utf-8")
-				const globalConfig = JSON.parse(globalContent)
+				const globalConfig = await safeReadJson(globalPath)
 				globalServers = globalConfig.mcpServers || {}
 				const globalServerNames = Object.keys(globalServers)
 			} catch (error) {
@@ -1248,8 +1242,7 @@ export class McpHub {
 			let projectServers: Record<string, any> = {}
 			if (projectPath) {
 				try {
-					const projectContent = await fs.readFile(projectPath, "utf-8")
-					const projectConfig = JSON.parse(projectContent)
+					const projectConfig = await safeReadJson(projectPath)
 					projectServers = projectConfig.mcpServers || {}
 					const projectServerNames = Object.keys(projectServers)
 				} catch (error) {
@@ -1281,8 +1274,7 @@ export class McpHub {
 	private async notifyWebviewOfServerChanges(): Promise<void> {
 		// Get global server order from settings file
 		const settingsPath = await this.getMcpSettingsFilePath()
-		const content = await fs.readFile(settingsPath, "utf-8")
-		const config = JSON.parse(content)
+		const config = await safeReadJson(settingsPath)
 		const globalServerOrder = Object.keys(config.mcpServers || {})
 
 		// Get project server order if available
@@ -1290,8 +1282,7 @@ export class McpHub {
 		let projectServerOrder: string[] = []
 		if (projectMcpPath) {
 			try {
-				const projectContent = await fs.readFile(projectMcpPath, "utf-8")
-				const projectConfig = JSON.parse(projectContent)
+				const projectConfig = await safeReadJson(projectMcpPath)
 				projectServerOrder = Object.keys(projectConfig.mcpServers || {})
 			} catch (error) {
 				// Silently continue with empty project server order
@@ -1429,8 +1420,9 @@ export class McpHub {
 		}
 
 		// Read and parse the config file
-		const content = await fs.readFile(configPath, "utf-8")
-		const config = JSON.parse(content)
+		// This is a read-modify-write-operation, but we cannot
+		// use safeWriteJson because it does not (yet) support pretty printing.
+		const config = await safeReadJson(configPath)
 
 		// Validate the config structure
 		if (!config || typeof config !== "object") {
@@ -1520,8 +1512,9 @@ export class McpHub {
 				throw new Error("Settings file not accessible")
 			}
 
-			const content = await fs.readFile(configPath, "utf-8")
-			const config = JSON.parse(content)
+			// This is a read-modify-write-operation, but we cannot
+			// use safeWriteJson because it does not (yet) support pretty printing.
+			const config = await safeReadJson(configPath)
 
 			// Validate the config structure
 			if (!config || typeof config !== "object") {
@@ -1658,8 +1651,9 @@ export class McpHub {
 		const normalizedPath = process.platform === "win32" ? configPath.replace(/\\/g, "/") : configPath
 
 		// Read the appropriate config file
-		const content = await fs.readFile(normalizedPath, "utf-8")
-		const config = JSON.parse(content)
+		// This is a read-modify-write-operation, but we cannot
+		// use safeWriteJson because it does not (yet) support pretty printing.
+		const config = await safeReadJson(configPath)
 
 		if (!config.mcpServers) {
 			config.mcpServers = {}

src/services/mdm/MdmService.ts

@@ -4,7 +4,7 @@ import * as os from "os"
 import { z } from "zod"
 
 import { CloudService, getClerkBaseUrl, PRODUCTION_CLERK_BASE_URL } from "@roo-code/cloud"
-
+import { safeReadJson } from "../../utils/safeReadJson"
 import { t } from "../../i18n"
 
 // MDM Configuration Schema
@@ -120,19 +120,16 @@ export class MdmService {
 		const configPath = this.getMdmConfigPath()
 
 		try {
-			// Check if file exists
-			if (!fs.existsSync(configPath)) {
-				return null
-			}
-
-			// Read and parse the configuration file
-			const configContent = fs.readFileSync(configPath, "utf-8")
-			const parsedConfig = JSON.parse(configContent)
+			// Read and parse the configuration file using safeReadJson
+			const parsedConfig = await safeReadJson(configPath)
 
 			// Validate against schema
 			return mdmConfigSchema.parse(parsedConfig)
 		} catch (error) {
-			this.log(`[MDM] Error reading MDM config from ${configPath}:`, error)
+			// If file doesn't exist, return null
+			if ((error as any)?.code !== "ENOENT") {
+				this.log(`[MDM] Error reading MDM config from ${configPath}:`, error)
+			}
 			return null
 		}
 	}