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>
This commit is contained in:
1 parent
ad9e22d05f
commit
ae433f0983
3 files changed
+90
-3
No files matched your search
+15
-3
@@ -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,
|
||||
}),
|
||||
};
|
||||
};
|
||||
|
||||
@@ -13,4 +13,5 @@ export const MCP_STARTUP_POLICY = {
|
||||
toolsListBudgetMs: 5_000,
|
||||
firstCatalogBudgetMs: 20_000,
|
||||
doctorDeadlineMs: 30_000,
|
||||
doctorStderrBytes: 64 * 1024,
|
||||
} as const;
|
||||
@@ -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<void> => {
|
||||
for (let attempt = 0; attempt < 50; attempt += 1) {
|
||||
try {
|
||||
|
||||
Reference in new issue
Block a user