fix(utils): Prevent RangeError from large command output#433
fix(utils): Prevent RangeError from large command output#433sentry[bot] wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
No tests cover the new output-truncation / maxOutputBytes behaviour
The core fix introduced by this PR (buffer accumulation, truncation cap, appendChunk, finalizeStream, XCODEBUILDMCP_MAX_OUTPUT_BYTES) has no corresponding tests in src/utils/__tests__/command.test.ts, leaving the crash-prevention logic unverified.
Evidence
src/utils/__tests__/command.test.tscontains only two tests (delayed close settlement and detached listener count) — neither exercises large output, the cap, or truncation.grepformaxOutputBytes,truncat,appendChunk,RangeErrorin that file returns no matches.- The PR description identifies a specific crash scenario (
RangeError: Invalid string length) that is the sole motivation for the change, but no regression test validates it.
Identified by Warden code-review
| @@ -165,8 +229,16 @@ async function defaultExecutor( | |||
| if (settled) { | |||
There was a problem hiding this comment.
No tests for the new buffer accumulation, capping, and truncation logic
The new appendChunk / finalizeStream functions and the maxOutputBytes cap are entirely untested — src/utils/__tests__/command.test.ts has no test that exercises large output, truncation, the [output truncated after N bytes] message, or the XCODEBUILDMCP_MAX_OUTPUT_BYTES env-var path. Consider adding unit tests that verify: (1) output below the cap is returned verbatim, (2) output at or above the cap is truncated with the expected message, and (3) the env-var override is respected.
Evidence
src/utils/__tests__/command.test.tscontains only two integration-style tests (settles after exitanddoes not attach stdout/stderr listeners for detached commands), neither of which setsmaxOutputBytesor generates output large enough to trigger truncation.appendChunk(the byte-cap logic) andfinalizeStream(the concatenation + truncation-message logic) are not called or validated by any test in the suite.- A grep across all
*.test.tsfiles formaxOutputBytes,appendChunk,finalizeStream, andtruncat(in the context of command output) returns zero matches. - The
XCODEBUILDMCP_MAX_OUTPUT_BYTESenv-var branch insidedefaultExecutorhas no test path covering it.
Also found at 1 additional location
src/utils/command.ts:76
Identified by Warden code-review · UJ6-YUH
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6155c40. Configure here.
| const safeSlice = Math.min(totalBytes, 1 * 1024 * 1024); | ||
| const safeText = Buffer.concat(chunks, totalBytes) | ||
| .subarray(0, safeSlice) | ||
| .toString('utf8'); |
There was a problem hiding this comment.
Catch block in finalizeStream can itself throw unhandled
Medium Severity
The defensive catch block in finalizeStream repeats Buffer.concat(chunks, totalBytes) without its own try/catch. If the original failure was Buffer.concat itself throwing (e.g., ERR_MEMORY_ALLOCATION_FAILED when maxOutputBytes is configured very large via the env var), this second call will also throw. Since finalizeStream is called from settle(), which is invoked from event handlers and setTimeout callbacks, the unhandled throw becomes an uncaught exception — the exact crash scenario this PR aims to prevent.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 6155c40. Configure here.


This PR addresses the
RangeError: Invalid string lengthissue (XCODEBUILDMCP-E) that caused the MCP process to crash whenxcodebuildor other commands produced very large outputs.Root Cause:
The
runCommandhelper insrc/utils/command.tsaccumulated child processstdoutandstderrby concatenating chunks into a single JavaScript string (stdout += data.toString()). When the output size exceeded V8's maximum string length (~512MB on 64-bit systems), aRangeError: Invalid string lengthwas thrown synchronously inside thedatahandler, leading to an uncaught exception and crashing the MCP server.Solution:
stdoutandstderrwith accumulation intoBuffer[]arrays. This avoids hitting V8's string length limit during data streaming.maxOutputBytesoption (defaulting to 64 MiB, configurable viaXCODEBUILDMCP_MAX_OUTPUT_BYTESenvironment variable orCommandExecOptions). Output exceeding this limit is truncated, and a truncation marker is appended.outputanderrorstrings are materialized from the accumulatedBuffer[]arrays usingBuffer.concat().toString('utf8')only when the command settles. A defensivetry/catchis in place during finalization to handle extreme edge cases, falling back to a smaller slice if concatenation still fails.datahandler now includes atry/catchblock. Any error (including potential futureRangeErrors) within the handler will now reject the command promise viahandleErrorinstead of causing an uncaught exception and crashing the process.CommandExecOptionsUpdate: AddedmaxOutputBytestoCommandExecOptionsfor explicit control over the output limit.Fixes XCODEBUILDMCP-E