Repository navigation
Match MCP JSON-RPC responses that echo string ids - #343
Merged
Merged
Conversation
JSON-RPC 2.0 allows String | Number ids. parse_response only copied numeric ids, so a server that echoed "id":"1" left Response::id at 0. Client::rpc then treated a valid reply as a notification and hung until timeout, killing the MCP subprocess. Co-authored-by: Tyler Reckart <tylerreckart@users.noreply.github.com>
This was referenced Sep 15, 2026
…c-ids # Conflicts: # CHANGELOG.md
tylerreckart
marked this pull request as ready for review
September 21, 2026 12:59
tylerreckart
enabled auto-merge (squash)
September 21, 2026 12:59
…c-ids # Conflicts: # CHANGELOG.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
parse_responseonly copied JSON-RPCidwhen the member was a number. JSON-RPC 2.0 allows String | Number | Null, and JS MCP stacks commonly echo the integer we sent as"id":"1".Client::rpcmatchesresp.id == req.id(req.idstarts at 1). A string id leftResponse::idat 0 — the same sentinel used for notifications — so a valid reply was skipped. The loop then sat untilinit_timeout/call_timeoutand killed the MCP subprocess. Playwright and other stdio MCP servers that stringify ids hung on everyinitialize/tools/call.Fix
Parse decimal-string ids with
stoll(full-string match, base 10) into the sameint64_twe already store for numeric ids. Null, empty, non-numeric, and fractional ("1.0") ids stay 0 so notifications and unmatched traffic still skip. Numeric ids are unchanged.Tests
unit_mcp— string success/error ids, non-numeric / empty / null /"1.0"skip, omitted id (notification). 4parse_response*cases locally; suite 48/48 with the pre-existingSubprocess: terminateSIGTERM flake excluded. ASan/UBSan driver overparse_responsepassed.Independently mergeable against
main(0456350). Code hunks (mcp/types.{h,cpp},test_mcp.cppinserted before the malformed-envelope case) auto-merge with #337 (null members, later inparse_response). CHANGELOG[Unreleased]will need a rebase after whichever sibling merges first.Note
Low Risk
Localized MCP JSON-RPC parsing change with tests; no auth or data-path impact, though incorrect id handling could still affect MCP subprocess RPC behavior.
Overview
Fixes MCP stdio hangs when JavaScript servers echo JSON-RPC request ids as strings (
"id":"3") instead of numbers.parse_responsenow normalizes decimal-string ids into the sameint64_tused for numeric ids via a newparse_jsonrpc_idhelper (stoll, full-string match). That letsClient::rpcmatch replies again instead of treating them as notifications (id == 0) and waiting until init/call timeout kills the subprocess. Empty, null, non-numeric, and fractional string ids still map to0so the notification skip path is unchanged. Unit tests cover success/error string ids and skip cases; the unreleased changelog documents the fix.Reviewed by Cursor Bugbot for commit edc6427. Bugbot is set up for automated code reviews on this repo. Configure here.