-
Notifications
You must be signed in to change notification settings - Fork 4
fix(mcp): relay upstream JSON-RPC error bodies through clerk mcp run #433
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "clerk": patch | ||
| --- | ||
|
|
||
| `clerk mcp run` now relays a structured JSON-RPC error from the upstream server verbatim instead of collapsing it into a generic -32000 error, so a client can see reserved codes like `HeaderMismatch` (-32020) and `UnsupportedProtocolVersion` (-32022) and drive the 2026-07-28 negotiation-retry flow. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -191,6 +191,13 @@ async function dispatch(message: JSONRPCMessage, ctx: DispatchCtx): Promise<void | |
| if (response.status === 202 || response.status === 204) return; | ||
|
|
||
| if (!response.ok) { | ||
| // A structured JSON-RPC error body (e.g. the MCP-reserved -32020..-32022 | ||
| // codes with `data.supported`) carries information the driving client | ||
| // needs — most importantly for the 2026-07-28 negotiation-retry flow. | ||
| // Relay it verbatim instead of collapsing it into a generic -32000. | ||
| // Notifications never get a reply, not even a relayed upstream error — | ||
| // emitError below already stays silent for them. | ||
| if ("id" in message && (await relayUpstreamError(response, emitPayload))) return; | ||
| await emitError(message, emit, -32000, `Upstream returned HTTP ${response.status}.`); | ||
| return; | ||
| } | ||
|
|
@@ -290,6 +297,40 @@ async function emitError( | |
| await emit({ jsonrpc: "2.0", id: message.id, error: { code, message: text } }); | ||
| } | ||
|
|
||
| /** | ||
| * Attempt to relay a non-ok upstream response as-is: read the body, and if it | ||
| * parses as a well-formed JSON-RPC error, forward it verbatim through the | ||
| * normal emit path. Returns `false` (nothing emitted) for a non-JSON body or | ||
| * JSON that isn't a JSON-RPC error, so the caller falls back to a generic | ||
| * -32000. | ||
| */ | ||
| async function relayUpstreamError(response: Response, emitPayload: Emit): Promise<boolean> { | ||
| let text: string | undefined; | ||
| try { | ||
| text = await readTextCapped(response, MAX_LINE_BYTES); | ||
| } catch { | ||
| // A body that dies mid-read is just an unreadable body — fall back rather | ||
| // than letting the rejection escape and take the whole bridge down. | ||
| return false; | ||
| } | ||
| if (text === undefined || text.trim().length === 0) return false; | ||
|
rafa-thayto marked this conversation as resolved.
|
||
| let parsed: unknown; | ||
| try { | ||
| parsed = JSON.parse(text); | ||
| } catch { | ||
| return false; | ||
| } | ||
| if (!isJsonRpcErrorResponse(parsed)) return false; | ||
| await emitPayload(parsed); | ||
| return true; | ||
| } | ||
|
|
||
| /** True when a parsed body is a well-formed JSON-RPC 2.0 error response. */ | ||
| function isJsonRpcErrorResponse(payload: unknown): boolean { | ||
| if (!isRecord(payload) || payload.jsonrpc !== "2.0" || !("id" in payload)) return false; | ||
| return isRecord(payload.error) && typeof payload.error.code === "number"; | ||
| } | ||
|
Comment on lines
+328
to
+332
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- relevant source ---'
sed -n '160,215p;285,340p' packages/cli-core/src/commands/mcp/run.ts
printf '%s\n' '--- schema references ---'
rg -n --glob '!node_modules' 'JSONRPCMessageSchema|`@modelcontextprotocol/sdk`' packages/cli-core package.json bun.lockb bun.lock yarn.lock package-lock.json 2>/dev/null || true
printf '%s\n' '--- related tests ---'
fd -i 'run.test.ts' . | xargs -r -n1 sh -c 'echo "--- $0"; rg -n -C 5 "JSONRPCMessageSchema|isJsonRpcErrorResponse|error response|notification" "$0" || true'
printf '%s\n' '--- dependency metadata ---'
rg -n -C 3 '"`@modelcontextprotocol/sdk`"|`@modelcontextprotocol/sdk`' . --glob 'package.json' --glob 'bun.lock*' --glob 'yarn.lock' --glob 'package-lock.json' --glob 'pnpm-lock.yaml' 2>/dev/null || trueRepository: clerk/cli Length of output: 13852 🏁 Script executed (no clone): Length of output: 14322 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- imports and relay tests ---'
sed -n '1,45p' packages/cli-core/src/commands/mcp/run.ts
rg -n -C 10 'relayUpstreamError|HTTP 4|HTTP 5|structured|generic -32000|non-JSON|message' packages/cli-core/src/commands/mcp/run.test.ts
printf '%s\n' '--- SDK export details ---'
curl -fsSL 'https://unpkg.com/@modelcontextprotocol/sdk@1.29.0/package.json' |
jq '{version, type, exports}'
curl -fsSL 'https://unpkg.com/@modelcontextprotocol/sdk@1.29.0/dist/esm/types.js' |
sed -n '168,218p'Repository: clerk/cli Length of output: 16380 🏁 Script executed (no clone): Length of output: 483 Validate upstream error responses with The predicate relays malformed bodies, including missing 🤖 Prompt for AI Agents |
||
|
|
||
| function requestHeaders(session: Session): Record<string, string> { | ||
| return { | ||
| "Content-Type": "application/json", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Match the relayed error ID to the request ID.
relayUpstreamErroraccepts any JSON-RPC error ID. If the upstream body has a different ID,emitPayloadwrites a frame that does not reply to this request and Line 200 skips the generic fallback. Passmessage.idinto the helper. Returnfalsewhen the IDs differ.Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents