[PR #3121] [CLOSED] fix(mcp): bound doctor startup stderr retention #5629

New issue

Closed

opened 2026-10-31 01:08:06 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:08:06 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3121
Author: @1052326311
Created: 10/14/2026
Status: ❌ Closed

Base: main ← Head: fix/mcp-doctor-stderr

📝 Commits (2)

9cb08bf fix(mcp): bound doctor startup stderr retention
f0b8529 Merge remote-tracking branch 'origin/main' into HEAD

📊 Changes

4 files changed (+113 additions, -6 deletions)

View changed files

📝 docs/cli.md (+8 -0)
📝 src/mcpDoctor.ts (+15 -6)
📝 src/mcpStartupPolicy.ts (+2 -0)
📝 tests/boundary/mcp/mcpDoctor.test.ts (+88 -0)

📄 Description

Summary

Bound rea mcp doctor startup stderr retention so a noisy child cannot grow the diagnostic buffer and final report without a byte limit. No separate issue: this was found while investigating diagnostic output during #3055 verification.

Problem and expected behavior

captureStderr previously copied every stderr chunk and concatenated the entire stream after the diagnostic completed. A startup deadline limits time, but does not bound accumulated log bytes. An owned failing child producing 65,537 bytes returned all 65,537; the new overflow regression fails on unchanged main 58300885.

The doctor should keep a bounded diagnostic preview, continue draining the child, and disclose omitted output without changing its startup/health decision.

Change and scope

Keep the first 64 KiB of raw stderr in one fixed buffer, with the retention budget in MCP_STARTUP_POLICY. Continue counting/draining all observed bytes. diagnostics.stderr_bytes retains its total-byte meaning; new stderr_truncated reports overflow. Decode the prefix with StringDecoder so cutting through a UTF-8 code point does not manufacture a replacement character. Existing replacement decoding remains for malformed untruncated logs.

The budget is an explicit diagnostic-retention resource policy, not an MCP message-size limit. The CLI guide documents it. The child transport, absolute deadline and joined cleanup are unchanged. This is separate from #2693's argument/envelope correction; its patch does not change the stderr collector.

Contract and boundary impact

Semantic owner and earliest changed stage: captureStderr, at child stderr receipt before copying/retaining bytes.
Contribution scope fit and linked maintainer decision: routine bounded-resource/diagnostic repair; a new-capability decision is not applicable.
CLI and MCP/tool-catalog contract: doctor diagnostic report adds stderr_truncated; short logs and total byte counts are preserved. No MCP tools/schema/catalog changes.
Provider, bridge, target-format, or platform compatibility: no provider or target changes; uses Node's existing Buffer/StringDecoder APIs.
Evidence, artifact, provenance, or reconstruction contract: no analysis Evidence changes; diagnostic omission is explicit.
Target selection, authority, upstream behavior, and cleanup: existing target-free MCP probe, ownership and shutdown retained; no added process authority.
Generated metadata, package, or installation impact: none.

Expected effects:

Launch or attach to a process.
Inject instrumentation or run scripts/code.

These describe the existing doctor launching its own target-free stdio child. The change only bounds its retained stderr; it adds no injection, target execution, installation or network behavior.

Evidence and regression coverage

Real child processes exercise empty, below/exact/over-budget output, complete multibyte text, a budget cutting a multibyte code point, and malformed untruncated UTF-8. Final JSON is checked both bare and with --full-output: a child writes 4,194,304 bytes across 64 awaited writes; the report retains 65,536 bytes, counts all 4,194,304 and sets stderr_truncated: true, while preserving unhealthy status/exit 1. Awaited writes test continued draining after the retention buffer fills.

The existing suite also verifies a healthy production MCP child against the actual catalog, schema-invalid replies, startup failure, and deadline-triggered cleanup. Initial baseline: 5 new assertions failed / 6 existing tests passed; both overflow cases failed on the actual excess report length. Final candidate: 15 tests passed.

Example report fragment for the 4 MiB fixture:

{"healthy":false,"diagnostics":{"stderr":"<65536 retained x bytes>","stderr_bytes":4194304,"stderr_truncated":true}}

The stderr placeholder above abbreviates the tested literal string. This proves bounded capture/reporting; no out-of-memory crash was induced or claimed.

Validation performed

On macOS arm64, Node 24.18.0 / npm 11.16.0, base 58300885, candidate 9cb08bfe:

npm ci --prefer-offline --no-audit --no-fund — locked install passed.
npm run test:focused -- tests/boundary/mcp/mcpDoctor.test.ts — 15 passed, including real production MCP startup and owned-child cleanup.
npm run check — passed: runtime build, typecheck, lint/module boundaries, formatting, metadata/unused-code and architecture guards.
npm run docs:check — passed, including 5 README consistency tests.
git diff --check and staged-content inspection — passed; normal commit/push hooks used.

Compatibility, safety, and release

No configuration or migration needed. Large diagnostic logs now return a disclosed prefix rather than the entire log; stderr_bytes remains the observed total. Malformed byte sequences retain replacement decoding, which can expand text relative to the raw-byte budget while remaining bounded. Windows/Linux execution and installed-package verification were not run; no native-engine changes or claims. Full hosted CI remains a post-publication gate.

Before merge

Concrete reproduced diagnostic resource problem and one focused outcome.
Existing scope and authority preserved.
Reason for no separate issue stated above.
Real owning-boundary regressions and final CLI JSON checked.
Healthy startup and cleanup controls retained.
Owning documentation and representative output included.
Final diff checked for secrets, generated output and unrelated cleanup.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/3121 **Author:** [@1052326311](https://github.com/1052326311) **Created:** 10/14/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/mcp-doctor-stderr` --- ### 📝 Commits (2) - [`9cb08bf`](https://github.com/morluto/rea/commit/9cb08bfe9da6344c198f2d03d6961620d89f45f7) fix(mcp): bound doctor startup stderr retention - [`f0b8529`](https://github.com/morluto/rea/commit/f0b8529ff6664f24c78a0d0460881eedebecf99f) Merge remote-tracking branch 'origin/main' into HEAD ### 📊 Changes **4 files changed** (+113 additions, -6 deletions) <details> <summary>View changed files</summary> 📝 `docs/cli.md` (+8 -0) 📝 `src/mcpDoctor.ts` (+15 -6) 📝 `src/mcpStartupPolicy.ts` (+2 -0) 📝 `tests/boundary/mcp/mcpDoctor.test.ts` (+88 -0) </details> ### 📄 Description ## Summary Bound `rea mcp doctor` startup stderr retention so a noisy child cannot grow the diagnostic buffer and final report without a byte limit. No separate issue: this was found while investigating diagnostic output during #3055 verification. ## Problem and expected behavior `captureStderr` previously copied every stderr chunk and concatenated the entire stream after the diagnostic completed. A startup deadline limits time, but does not bound accumulated log bytes. An owned failing child producing 65,537 bytes returned all 65,537; the new overflow regression fails on unchanged main `58300885`. The doctor should keep a bounded diagnostic preview, continue draining the child, and disclose omitted output without changing its startup/health decision. ## Change and scope Keep the first 64 KiB of raw stderr in one fixed buffer, with the retention budget in `MCP_STARTUP_POLICY`. Continue counting/draining all observed bytes. `diagnostics.stderr_bytes` retains its total-byte meaning; new `stderr_truncated` reports overflow. Decode the prefix with `StringDecoder` so cutting through a UTF-8 code point does not manufacture a replacement character. Existing replacement decoding remains for malformed untruncated logs. The budget is an explicit diagnostic-retention resource policy, not an MCP message-size limit. The CLI guide documents it. The child transport, absolute deadline and joined cleanup are unchanged. This is separate from #2693's argument/envelope correction; its patch does not change the stderr collector. ## Contract and boundary impact - Semantic owner and earliest changed stage: `captureStderr`, at child stderr receipt before copying/retaining bytes. - Contribution scope fit and linked maintainer decision: routine bounded-resource/diagnostic repair; a new-capability decision is not applicable. - CLI and MCP/tool-catalog contract: doctor diagnostic report adds `stderr_truncated`; short logs and total byte counts are preserved. No MCP tools/schema/catalog changes. - Provider, bridge, target-format, or platform compatibility: no provider or target changes; uses Node's existing Buffer/StringDecoder APIs. - Evidence, artifact, provenance, or reconstruction contract: no analysis Evidence changes; diagnostic omission is explicit. - Target selection, authority, upstream behavior, and cleanup: existing target-free MCP probe, ownership and shutdown retained; no added process authority. - Generated metadata, package, or installation impact: none. Expected effects: - [x] Launch or attach to a process. - [x] Inject instrumentation or run scripts/code. These describe the existing doctor launching its own target-free stdio child. The change only bounds its retained stderr; it adds no injection, target execution, installation or network behavior. ## Evidence and regression coverage Real child processes exercise empty, below/exact/over-budget output, complete multibyte text, a budget cutting a multibyte code point, and malformed untruncated UTF-8. Final JSON is checked both bare and with `--full-output`: a child writes 4,194,304 bytes across 64 awaited writes; the report retains 65,536 bytes, counts all 4,194,304 and sets `stderr_truncated: true`, while preserving unhealthy status/exit 1. Awaited writes test continued draining after the retention buffer fills. The existing suite also verifies a healthy production MCP child against the actual catalog, schema-invalid replies, startup failure, and deadline-triggered cleanup. Initial baseline: 5 new assertions failed / 6 existing tests passed; both overflow cases failed on the actual excess report length. Final candidate: 15 tests passed. Example report fragment for the 4 MiB fixture: ```json {"healthy":false,"diagnostics":{"stderr":"<65536 retained x bytes>","stderr_bytes":4194304,"stderr_truncated":true}} ``` The `stderr` placeholder above abbreviates the tested literal string. This proves bounded capture/reporting; no out-of-memory crash was induced or claimed. ## Validation performed On macOS arm64, Node 24.18.0 / npm 11.16.0, base `58300885`, candidate `9cb08bfe`: - `npm ci --prefer-offline --no-audit --no-fund` — locked install passed. - `npm run test:focused -- tests/boundary/mcp/mcpDoctor.test.ts` — 15 passed, including real production MCP startup and owned-child cleanup. - `npm run check` — passed: runtime build, typecheck, lint/module boundaries, formatting, metadata/unused-code and architecture guards. - `npm run docs:check` — passed, including 5 README consistency tests. - `git diff --check` and staged-content inspection — passed; normal commit/push hooks used. ## Compatibility, safety, and release No configuration or migration needed. Large diagnostic logs now return a disclosed prefix rather than the entire log; `stderr_bytes` remains the observed total. Malformed byte sequences retain replacement decoding, which can expand text relative to the raw-byte budget while remaining bounded. Windows/Linux execution and installed-package verification were not run; no native-engine changes or claims. Full hosted CI remains a post-publication gate. ## Before merge - [x] Concrete reproduced diagnostic resource problem and one focused outcome. - [x] Existing scope and authority preserved. - [x] Reason for no separate issue stated above. - [x] Real owning-boundary regressions and final CLI JSON checked. - [x] Healthy startup and cleanup controls retained. - [x] Owning documentation and representative output included. - [x] Final diff checked for secrets, generated output and unrelated cleanup. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:08:06 +00:00