From ae433f0983acfc320c4be1cc203931a8a1e971dd Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 14 Oct 2026 13:58:17 +0800 Subject: [PATCH] fix(mcp): bound retained doctor stderr while draining output Retain a 64 KiB diagnostic prefix while counting and draining the complete stderr stream. Report truncation and avoid manufacturing a replacement character at a truncated UTF-8 boundary. Validation: all nine doctor tests pass, including real noisy child, malformed bytes, canonical catalog and startup deadline controls. Based on #3121. Co-authored-by: 1052326311 <65798732+1052326311@users.noreply.github.com> --- src/mcpDoctor.ts | 18 +++++-- src/mcpStartupPolicy.ts | 1 + tests/boundary/mcp/mcpDoctor.test.ts | 74 ++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+), 3 deletions(-) diff --git a/src/mcpDoctor.ts b/src/mcpDoctor.ts index ee2529ff12..75859eb270 100644 --- a/src/mcpDoctor.ts +++ b/src/mcpDoctor.ts @@ -3,6 +3,7 @@ import { StdioClientTransport } from "@modelcontextprotocol/client/stdio"; import { Formatter, Help, z } from "incur"; import { performance } from "node:perf_hooks"; import { resolve } from "node:path"; +import { StringDecoder } from "node:string_decoder"; import { CATALOG_IDENTITY } from "./catalogIdentity.js"; import { PROMPT_CONTRACTS } from "./contracts/promptContracts.js"; @@ -483,19 +484,30 @@ const targetFreeEnvironment = ( }; const captureStderr = () => { - const chunks: Buffer[] = []; + const retained = Buffer.allocUnsafe(MCP_STARTUP_POLICY.doctorStderrBytes); + let retainedBytes = 0; let bytes = 0; + let truncated = false; return { append(chunk: unknown) { const buffer = Buffer.isBuffer(chunk) ? chunk : Buffer.from(String(chunk), "utf8"); bytes += buffer.byteLength; - chunks.push(Buffer.from(buffer)); + const remaining = retained.byteLength - retainedBytes; + const copied = Math.min(remaining, buffer.byteLength); + if (copied > 0) { + buffer.copy(retained, retainedBytes, 0, copied); + retainedBytes += copied; + } + if (copied < buffer.byteLength) truncated = true; }, value: () => ({ - stderr: Buffer.concat(chunks, bytes).toString("utf8"), + stderr: truncated + ? new StringDecoder("utf8").write(retained.subarray(0, retainedBytes)) + : retained.toString("utf8", 0, retainedBytes), stderr_bytes: bytes, + stderr_truncated: truncated, }), }; }; diff --git a/src/mcpStartupPolicy.ts b/src/mcpStartupPolicy.ts index dc31114c1d..8eea0c6dfe 100644 --- a/src/mcpStartupPolicy.ts +++ b/src/mcpStartupPolicy.ts @@ -13,4 +13,5 @@ export const MCP_STARTUP_POLICY = { toolsListBudgetMs: 5_000, firstCatalogBudgetMs: 20_000, doctorDeadlineMs: 30_000, + doctorStderrBytes: 64 * 1024, } as const; diff --git a/tests/boundary/mcp/mcpDoctor.test.ts b/tests/boundary/mcp/mcpDoctor.test.ts index acfb2bb0e6..31afb30ace 100644 --- a/tests/boundary/mcp/mcpDoctor.test.ts +++ b/tests/boundary/mcp/mcpDoctor.test.ts @@ -13,6 +13,7 @@ import { runProductionMcpDoctorCli, } from "../../../src/mcpDoctor.js"; import { CATALOG_IDENTITY } from "../../../src/catalogIdentity.js"; +import { MCP_STARTUP_POLICY } from "../../../src/mcpStartupPolicy.js"; afterEach(() => vi.useRealTimers()); @@ -170,6 +171,66 @@ describe("production MCP doctor child lifecycle", () => { }); }); + it("retains only the stderr prefix while draining and counting the full child stream", async () => { + const result = await runNoisyDoctorChild(` + for (let index = 0; index < 64; index += 1) { + await new Promise((resolve, reject) => { + process.stderr.write(Buffer.alloc(65_536, 0x78), (error) => + error ? reject(error) : resolve(), + ); + }); + } + process.exitCode = 1; + `); + + expect(result.healthy).toBe(false); + expect(result.diagnostics).toEqual({ + stderr: "x".repeat(MCP_STARTUP_POLICY.doctorStderrBytes), + stderr_bytes: 4 * 1024 * 1024, + stderr_truncated: true, + }); + }, 10_000); + + it("does not manufacture a replacement character when the retained prefix ends inside UTF-8", async () => { + const prefixBytes = MCP_STARTUP_POLICY.doctorStderrBytes - 1; + const result = await runNoisyDoctorChild(` + for (const chunk of [ + Buffer.alloc(${prefixBytes}, 0x78), + Buffer.from([0xe2, 0x82, 0xac]), + ]) { + await new Promise((resolve, reject) => { + process.stderr.write(chunk, (error) => + error ? reject(error) : resolve(), + ); + }); + } + process.exitCode = 1; + `); + + expect(result.diagnostics).toEqual({ + stderr: "x".repeat(prefixBytes), + stderr_bytes: prefixBytes + 3, + stderr_truncated: true, + }); + }); + + it("preserves replacement decoding for malformed UTF-8 within an untruncated stream", async () => { + const result = await runNoisyDoctorChild(` + await new Promise((resolve, reject) => { + process.stderr.write(Buffer.from([0xff]), (error) => + error ? reject(error) : resolve(), + ); + }); + process.exitCode = 1; + `); + + expect(result.diagnostics).toEqual({ + stderr: "�", + stderr_bytes: 1, + stderr_truncated: false, + }); + }); + it("kills a child that misses the absolute startup deadline", async () => { const root = await createTestTempDirectory("rea-mcp-doctor-"); const pidPath = join(root, "pid"); @@ -200,6 +261,19 @@ describe("production MCP doctor child lifecycle", () => { }, 10_000); }); +const runNoisyDoctorChild = async (body: string) => { + const root = await createTestTempDirectory("rea-mcp-doctor-stderr-"); + const scriptPath = join(root, "noisy.mjs"); + await writeFile(scriptPath, body); + return runProductionMcpDoctor({ + command: process.execPath, + args: [scriptPath], + cwd: root, + environment: process.env, + deadlineMs: 5_000, + }); +}; + const waitForExit = async (pid: number): Promise => { for (let attempt = 0; attempt < 50; attempt += 1) { try {