fix: correct MCP stdio JSON-RPC framing and handshake - #6
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
|
||
| case "$method" in | ||
| "initialize") | ||
| printf '{"jsonrpc":"2.0","id":%s,"result":{"protocolVersion":"2024-11-05","capabilities":{"tools":{}},"serverInfo":{"name":"agentic-dns","version":"1.0.0"}}}\n' "$req_id" |
There was a problem hiding this comment.
📝 Info: String ids now framed as valid JSON
Switching .id // 1 from jq -r to jq -c quotes string ids, keeping every response valid JSON; numeric ids are unaffected and all branches stay consistent.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in commit ad8e278. Changed the id extraction from jq -r to jq -c so string ids (e.g. "init-1") stay JSON-encoded with quotes intact, producing valid "id":"init-1" instead of the broken "id":init-1. Verified with a string-id smoke test and a numeric-id regression test; applied to both the repo bin/agentic-dns and the live /root/hermes-dns/bin/hermes-dns copy.
|
|
||
| # ---- Helper functions ---- | ||
| log() { echo "[$(date '+%H:%M:%S')] $*"; } | ||
| log() { echo "[$(date '+%H:%M:%S')] $*" >&2; } |
There was a problem hiding this comment.
📝 Info: log() to stderr also affects the API server
Routing log to stderr also moves the API server's startup lines off stdout. Benign visibility change, worth noting since it affects a path beyond MCP.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Acknowledged, and intentional. The API server logs going to stderr is the correct convention (stdout stays clean for protocol/data output, diagnostics go to stderr). No behavior change for the API server itself - only which stream the startup banner uses. Leaving as-is.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| proto=$(echo "$line" | jq -c '.params.protocolVersion // "2024-11-05"' 2>/dev/null) | ||
| printf '{"jsonrpc":"2.0","id":%s,"result":{"protocolVersion":%s,"capabilities":{"tools":{}},"serverInfo":{"name":"agentic-dns","version":"1.0.0"}}}\n' "$req_id" "$proto" |
There was a problem hiding this comment.
📝 Info: protocolVersion echoed back without negotiation
The initialize handler echoes the client's requested protocolVersion back verbatim rather than replying with a version the server supports. The server thus claims to support any version string a client sends. Low impact for this tools-only server.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
The MCP server (
bin/agentic-dns mcp) was not actually MCP-compliant and would crash MCP Inspector, Cursor, and Claude Code on the handshake. This PR fixes six defects, verified against the official MCP Inspector CLI.Fixes
log()wrote to stdout, leaking[HH:MM:SS] Starting...lines into the JSON-RPC stream before the first frame. Now routes to stderr.initializehandshake — returnedMethod not supported: initialize, killing every real MCP client. Added a properinitializehandler (returnsprotocolVersion,capabilities,serverInfo).notifications/*handling — added a no-op branch.dns_queryresult unescaped — injected raw into the JSON string; a quote/newline in a DNS answer corrupted the frame. Now routed throughjq -Rs."input"instead of MCP-standard"inputSchema"with"type":"object"/"properties", so Inspector rendered no argument fields.tools/listoutput — the heredoc split the JSON array across physical lines, breaking newline-delimited stdio framing. Now emitted as a single line viaprintf.Also removed the phantom
dns_add_route/dns_remove_routetool names from the startup log (they were never intools/listor the dispatch switch).Verification
bash -n bin/agentic-dns— cleantests/test_cli.sh— all 4 tests pass@modelcontextprotocol/inspector):initializereturnsserverInfo,tools/listreturns 7 tools withinputSchema,tools/call dns_queryreturns a resolved IP.