Prechádzať zdrojové kódy

fix: use json-stream-stringify for pretty-printing MCP config files (#9864)

Co-authored-by: Roo Code <[email protected]>
Michaelzag 2 týždňov pred
rodič
commit
4e67357ab5

+ 10 - 1
pnpm-lock.yaml

@@ -853,6 +853,9 @@ importers:
       isbinaryfile:
         specifier: ^5.0.2
         version: 5.0.4
+      json-stream-stringify:
+        specifier: ^3.1.6
+        version: 3.1.6
       jwt-decode:
         specifier: ^4.0.0
         version: 4.0.0
@@ -7312,6 +7315,10 @@ packages:
   [email protected]:
     resolution: {integrity: sha512-Bdboy+l7tA3OGW6FjyFHWkP5LuByj1Tk33Ljyq0axyzdk9//JSi2u3fP1QSmd1KNwq6VOKYGlAu87CisVir6Pw==}
 
+  [email protected]:
+    resolution: {integrity: sha512-x7fpwxOkbhFCaJDJ8vb1fBY3DdSa4AlITaz+HHILQJzdPMnHEFjxPwVUi1ALIbcIxDE0PNe/0i7frnY8QnBQog==}
+    engines: {node: '>=7.10.1'}
+
   [email protected]:
     resolution: {integrity: sha512-ZClg6AaYvamvYEE82d3Iyd3vSSIjQ+odgjaTzRuO3s7toCdFKczob2i0zCh7JE8kWn17yvAWhUVxvqGwUalsRA==}
 
@@ -14537,7 +14544,7 @@ snapshots:
       sirv: 3.0.1
       tinyglobby: 0.2.14
       tinyrainbow: 2.0.0
-      vitest: 3.2.4(@types/[email protected])(@types/node@24.2.1)(@vitest/[email protected])([email protected])([email protected])([email protected])([email protected])([email protected])
+      vitest: 3.2.4(@types/[email protected])(@types/node@20.17.50)(@vitest/[email protected])([email protected])([email protected])([email protected])([email protected])([email protected])
 
   '@vitest/[email protected]':
     dependencies:
@@ -17673,6 +17680,8 @@ snapshots:
 
   [email protected]: {}
 
+  [email protected]: {}
+
   [email protected]: {}
 
   [email protected]: {}

+ 1 - 1
src/core/webview/webviewMessageHandler.ts

@@ -1358,7 +1358,7 @@ export const webviewMessageHandler = async (
 				const exists = await fileExistsAtPath(mcpPath)
 
 				if (!exists) {
-					await safeWriteJson(mcpPath, { mcpServers: {} })
+					await safeWriteJson(mcpPath, { mcpServers: {} }, { prettyPrint: true })
 				}
 
 				await openFile(mcpPath)

+ 1 - 0
src/package.json

@@ -490,6 +490,7 @@
 		"i18next": "^25.0.0",
 		"ignore": "^7.0.3",
 		"isbinaryfile": "^5.0.2",
+		"json-stream-stringify": "^3.1.6",
 		"jwt-decode": "^4.0.0",
 		"lodash.debounce": "^4.0.8",
 		"mammoth": "^1.9.1",

+ 3 - 3
src/services/mcp/McpHub.ts

@@ -1580,7 +1580,7 @@ export class McpHub {
 		}
 		this.isProgrammaticUpdate = true
 		try {
-			await safeWriteJson(configPath, updatedConfig)
+			await safeWriteJson(configPath, updatedConfig, { prettyPrint: true })
 		} finally {
 			// Reset flag after watcher debounce period (non-blocking)
 			this.flagResetTimer = setTimeout(() => {
@@ -1665,7 +1665,7 @@ export class McpHub {
 					mcpServers: config.mcpServers,
 				}
 
-				await safeWriteJson(configPath, updatedConfig)
+				await safeWriteJson(configPath, updatedConfig, { prettyPrint: true })
 
 				// Update server connections with the correct source
 				await this.updateServerConnections(config.mcpServers, serverSource)
@@ -1816,7 +1816,7 @@ export class McpHub {
 		}
 		this.isProgrammaticUpdate = true
 		try {
-			await safeWriteJson(normalizedPath, config)
+			await safeWriteJson(normalizedPath, config, { prettyPrint: true })
 		} finally {
 			// Reset flag after watcher debounce period (non-blocking)
 			this.flagResetTimer = setTimeout(() => {

+ 33 - 45
src/utils/safeWriteJson.ts

@@ -2,8 +2,20 @@ import * as fs from "fs/promises"
 import * as fsSync from "fs"
 import * as path from "path"
 import * as lockfile from "proper-lockfile"
-import Disassembler from "stream-json/Disassembler"
-import Stringer from "stream-json/Stringer"
+import { JsonStreamStringify } from "json-stream-stringify"
+
+/**
+ * Options for safeWriteJson function
+ */
+export interface SafeWriteJsonOptions {
+	/**
+	 * Whether to pretty-print the JSON output with indentation.
+	 * When true, uses tab characters for indentation.
+	 * When false or undefined, outputs compact JSON.
+	 * @default false
+	 */
+	prettyPrint?: boolean
+}
 
 /**
  * Safely writes JSON data to a file.
@@ -12,13 +24,15 @@ import Stringer from "stream-json/Stringer"
  * - Writes to a temporary file first.
  * - If the target file exists, it's backed up before being replaced.
  * - Attempts to roll back and clean up in case of errors.
+ * - Supports pretty-printing with indentation while maintaining streaming efficiency.
  *
  * @param {string} filePath - The absolute path to the target file.
  * @param {any} data - The data to serialize to JSON and write.
+ * @param {SafeWriteJsonOptions} options - Optional configuration for JSON formatting.
  * @returns {Promise<void>}
  */
 
-async function safeWriteJson(filePath: string, data: any): Promise<void> {
+async function safeWriteJson(filePath: string, data: any, options?: SafeWriteJsonOptions): Promise<void> {
 	const absoluteFilePath = path.resolve(filePath)
 	let releaseLock = async () => {} // Initialized to a no-op
 
@@ -75,7 +89,7 @@ async function safeWriteJson(filePath: string, data: any): Promise<void> {
 			`.${path.basename(absoluteFilePath)}.new_${Date.now()}_${Math.random().toString(36).substring(2)}.tmp`,
 		)
 
-		await _streamDataToFile(actualTempNewFilePath, data)
+		await _streamDataToFile(actualTempNewFilePath, data, options?.prettyPrint)
 
 		// Step 2: Check if the target file exists. If so, rename it to a backup path.
 		try {
@@ -182,53 +196,27 @@ async function safeWriteJson(filePath: string, data: any): Promise<void> {
  * Helper function to stream JSON data to a file.
  * @param targetPath The path to write the stream to.
  * @param data The data to stream.
+ * @param prettyPrint Whether to format the JSON with indentation.
  * @returns Promise<void>
  */
-async function _streamDataToFile(targetPath: string, data: any): Promise<void> {
+async function _streamDataToFile(targetPath: string, data: any, prettyPrint = false): Promise<void> {
 	// Stream data to avoid high memory usage for large JSON objects.
 	const fileWriteStream = fsSync.createWriteStream(targetPath, { encoding: "utf8" })
-	const disassembler = Disassembler.disassembler()
-	// Output will be compact JSON as standard Stringer is used.
-	const stringer = Stringer.stringer()
-
-	return new Promise<void>((resolve, reject) => {
-		let errorOccurred = false
-		const handleError = (_streamName: string) => (err: Error) => {
-			if (!errorOccurred) {
-				errorOccurred = true
-				if (!fileWriteStream.destroyed) {
-					fileWriteStream.destroy(err)
-				}
-				reject(err)
-			}
-		}
 
-		disassembler.on("error", handleError("Disassembler"))
-		stringer.on("error", handleError("Stringer"))
-		fileWriteStream.on("error", (err: Error) => {
-			if (!errorOccurred) {
-				errorOccurred = true
-				reject(err)
-			}
-		})
-
-		fileWriteStream.on("finish", () => {
-			if (!errorOccurred) {
-				resolve()
-			}
-		})
-
-		disassembler.pipe(stringer).pipe(fileWriteStream)
+	// JsonStreamStringify traverses the object and streams tokens directly
+	// The 'spaces' parameter adds indentation during streaming, not via a separate pass
+	// Convert undefined to null for valid JSON serialization (undefined is not valid JSON)
+	const stringifyStream = new JsonStreamStringify(
+		data === undefined ? null : data,
+		undefined, // replacer
+		prettyPrint ? "\t" : undefined, // spaces for indentation
+	)
 
-		// stream-json's Disassembler might error if `data` is undefined.
-		// JSON.stringify(undefined) would produce the string "undefined" if it's the root value.
-		// Writing 'null' is a safer JSON representation for a root undefined value.
-		if (data === undefined) {
-			disassembler.write(null)
-		} else {
-			disassembler.write(data)
-		}
-		disassembler.end()
+	return new Promise<void>((resolve, reject) => {
+		stringifyStream.on("error", reject)
+		fileWriteStream.on("error", reject)
+		fileWriteStream.on("finish", resolve)
+		stringifyStream.pipe(fileWriteStream)
 	})
 }