Skip to content

Commit 2e771f9

Browse files
Michaelzagroomote
authored andcommitted
fix: use json-stream-stringify for pretty-printing MCP config files (RooCodeInc#9864)
Co-authored-by: Roo Code <roomote@roocode.com>
1 parent 2953ece commit 2e771f9

5 files changed

Lines changed: 48 additions & 50 deletions

File tree

pnpm-lock.yaml

Lines changed: 10 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/core/webview/webviewMessageHandler.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1358,7 +1358,7 @@ export const webviewMessageHandler = async (
13581358
const exists = await fileExistsAtPath(mcpPath)
13591359

13601360
if (!exists) {
1361-
await safeWriteJson(mcpPath, { mcpServers: {} })
1361+
await safeWriteJson(mcpPath, { mcpServers: {} }, { prettyPrint: true })
13621362
}
13631363

13641364
await openFile(mcpPath)

src/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,7 @@
488488
"i18next": "^25.0.0",
489489
"ignore": "^7.0.3",
490490
"isbinaryfile": "^5.0.2",
491+
"json-stream-stringify": "^3.1.6",
491492
"jwt-decode": "^4.0.0",
492493
"lodash.debounce": "^4.0.8",
493494
"mammoth": "^1.9.1",

src/services/mcp/McpHub.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1580,7 +1580,7 @@ export class McpHub {
15801580
}
15811581
this.isProgrammaticUpdate = true
15821582
try {
1583-
await safeWriteJson(configPath, updatedConfig)
1583+
await safeWriteJson(configPath, updatedConfig, { prettyPrint: true })
15841584
} finally {
15851585
// Reset flag after watcher debounce period (non-blocking)
15861586
this.flagResetTimer = setTimeout(() => {
@@ -1665,7 +1665,7 @@ export class McpHub {
16651665
mcpServers: config.mcpServers,
16661666
}
16671667

1668-
await safeWriteJson(configPath, updatedConfig)
1668+
await safeWriteJson(configPath, updatedConfig, { prettyPrint: true })
16691669

16701670
// Update server connections with the correct source
16711671
await this.updateServerConnections(config.mcpServers, serverSource)
@@ -1816,7 +1816,7 @@ export class McpHub {
18161816
}
18171817
this.isProgrammaticUpdate = true
18181818
try {
1819-
await safeWriteJson(normalizedPath, config)
1819+
await safeWriteJson(normalizedPath, config, { prettyPrint: true })
18201820
} finally {
18211821
// Reset flag after watcher debounce period (non-blocking)
18221822
this.flagResetTimer = setTimeout(() => {

src/utils/safeWriteJson.ts

Lines changed: 33 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,20 @@ import * as fs from "fs/promises"
22
import * as fsSync from "fs"
33
import * as path from "path"
44
import * as lockfile from "proper-lockfile"
5-
import Disassembler from "stream-json/Disassembler"
6-
import Stringer from "stream-json/Stringer"
5+
import { JsonStreamStringify } from "json-stream-stringify"
6+
7+
/**
8+
* Options for safeWriteJson function
9+
*/
10+
export interface SafeWriteJsonOptions {
11+
/**
12+
* Whether to pretty-print the JSON output with indentation.
13+
* When true, uses tab characters for indentation.
14+
* When false or undefined, outputs compact JSON.
15+
* @default false
16+
*/
17+
prettyPrint?: boolean
18+
}
719

820
/**
921
* Safely writes JSON data to a file.
@@ -12,13 +24,15 @@ import Stringer from "stream-json/Stringer"
1224
* - Writes to a temporary file first.
1325
* - If the target file exists, it's backed up before being replaced.
1426
* - Attempts to roll back and clean up in case of errors.
27+
* - Supports pretty-printing with indentation while maintaining streaming efficiency.
1528
*
1629
* @param {string} filePath - The absolute path to the target file.
1730
* @param {any} data - The data to serialize to JSON and write.
31+
* @param {SafeWriteJsonOptions} options - Optional configuration for JSON formatting.
1832
* @returns {Promise<void>}
1933
*/
2034

21-
async function safeWriteJson(filePath: string, data: any): Promise<void> {
35+
async function safeWriteJson(filePath: string, data: any, options?: SafeWriteJsonOptions): Promise<void> {
2236
const absoluteFilePath = path.resolve(filePath)
2337
let releaseLock = async () => {} // Initialized to a no-op
2438

@@ -75,7 +89,7 @@ async function safeWriteJson(filePath: string, data: any): Promise<void> {
7589
`.${path.basename(absoluteFilePath)}.new_${Date.now()}_${Math.random().toString(36).substring(2)}.tmp`,
7690
)
7791

78-
await _streamDataToFile(actualTempNewFilePath, data)
92+
await _streamDataToFile(actualTempNewFilePath, data, options?.prettyPrint)
7993

8094
// Step 2: Check if the target file exists. If so, rename it to a backup path.
8195
try {
@@ -182,53 +196,27 @@ async function safeWriteJson(filePath: string, data: any): Promise<void> {
182196
* Helper function to stream JSON data to a file.
183197
* @param targetPath The path to write the stream to.
184198
* @param data The data to stream.
199+
* @param prettyPrint Whether to format the JSON with indentation.
185200
* @returns Promise<void>
186201
*/
187-
async function _streamDataToFile(targetPath: string, data: any): Promise<void> {
202+
async function _streamDataToFile(targetPath: string, data: any, prettyPrint = false): Promise<void> {
188203
// Stream data to avoid high memory usage for large JSON objects.
189204
const fileWriteStream = fsSync.createWriteStream(targetPath, { encoding: "utf8" })
190-
const disassembler = Disassembler.disassembler()
191-
// Output will be compact JSON as standard Stringer is used.
192-
const stringer = Stringer.stringer()
193-
194-
return new Promise<void>((resolve, reject) => {
195-
let errorOccurred = false
196-
const handleError = (_streamName: string) => (err: Error) => {
197-
if (!errorOccurred) {
198-
errorOccurred = true
199-
if (!fileWriteStream.destroyed) {
200-
fileWriteStream.destroy(err)
201-
}
202-
reject(err)
203-
}
204-
}
205205

206-
disassembler.on("error", handleError("Disassembler"))
207-
stringer.on("error", handleError("Stringer"))
208-
fileWriteStream.on("error", (err: Error) => {
209-
if (!errorOccurred) {
210-
errorOccurred = true
211-
reject(err)
212-
}
213-
})
214-
215-
fileWriteStream.on("finish", () => {
216-
if (!errorOccurred) {
217-
resolve()
218-
}
219-
})
220-
221-
disassembler.pipe(stringer).pipe(fileWriteStream)
206+
// JsonStreamStringify traverses the object and streams tokens directly
207+
// The 'spaces' parameter adds indentation during streaming, not via a separate pass
208+
// Convert undefined to null for valid JSON serialization (undefined is not valid JSON)
209+
const stringifyStream = new JsonStreamStringify(
210+
data === undefined ? null : data,
211+
undefined, // replacer
212+
prettyPrint ? "\t" : undefined, // spaces for indentation
213+
)
222214

223-
// stream-json's Disassembler might error if `data` is undefined.
224-
// JSON.stringify(undefined) would produce the string "undefined" if it's the root value.
225-
// Writing 'null' is a safer JSON representation for a root undefined value.
226-
if (data === undefined) {
227-
disassembler.write(null)
228-
} else {
229-
disassembler.write(data)
230-
}
231-
disassembler.end()
215+
return new Promise<void>((resolve, reject) => {
216+
stringifyStream.on("error", reject)
217+
fileWriteStream.on("error", reject)
218+
fileWriteStream.on("finish", resolve)
219+
stringifyStream.pipe(fileWriteStream)
232220
})
233221
}
234222

0 commit comments

Comments
 (0)