feat: settle MCP payments in the payment-aware fetch#567
Conversation
commit: |
6423f4a to
bd3001d
Compare
| @@ -0,0 +1,14 @@ | |||
| const mcpChallenge = Symbol.for('mppx.mcp.challenge') | |||
There was a problem hiding this comment.
intentionally modeling this as a "challenge brand" similar to how we treat x402 within normal http transport
| * this transport only adapts the HTTP body. The body is read only when the *request* is a | ||
| * JSON-RPC message, so non-MCP traffic (e.g. a streaming LLM call) is never read. | ||
| */ | ||
| export function http() { |
There was a problem hiding this comment.
I think longer term we need to break out / refactor the different transports, since they are getting overloaded and now are misnomers. But since fetch and others default to the http transport, the most minimal change rn is to just overload this transport even more.
There was a problem hiding this comment.
What do you envision here?
Technically http is the right transport, but today we do mix multiple protocols (rest, mcp, etc...) inside the code which is a bit gnarly
IMO it may make sense to have a pluggable "adapter" so that you can add new protocols to a transport easily?
There was a problem hiding this comment.
Yeah exactly. Like http is the right transport, but then we also have an mcp transport (which is a protocol, not a transport). Agree with the pluggable adapter.
There was a problem hiding this comment.
This would get rid of the challenge branding stuff too. We would just attempt to dispatch the request/response to all the adapters greedily choose the first one that fits. Should we bundle those changes in this PR?
141429a to
e28e494
Compare
e28e494 to
0337a82
Compare
| @@ -0,0 +1,79 @@ | |||
| import * as Mcp from '../../../Mcp.js' | |||
| import type { Adapter, Transport } from '../../Transport.js' | |||
| import { paymentRequiredStatus } from './Shared.js' | |||
There was a problem hiding this comment.
instead of adapters can we call these protocols?
There was a problem hiding this comment.
e.g. src/client/internal/protocols
we should also comment what these are -- it's not super apparent unless you are familiar with convos on this pr
| */ | ||
| export function adapter(): Adapter { | ||
| return { | ||
| name: 'mpp', |
There was a problem hiding this comment.
given that we ar pulling from shard anyway, can we make these names constants?
6134ccf to
bfcd9c1
Compare
Transport.http() now handles MCP-over-HTTP as a third HTTP payment flavor: a JSON-RPC -32042 error in a normal 200 body (often text/event-stream), credential carried in _meta. The -32042/_meta protocol is delegated to the existing Transport.mcp() codec; http() only adapts the HTTP body, and only reads it when the request is a JSON-RPC message. MCP challenges are routed at attach time via a Symbol brand mirroring x402. Transport.mcp() is unchanged and transport stays the single pluggable seam; non-breaking.
bfcd9c1 to
e8df79b
Compare
brendanjryan
left a comment
There was a problem hiding this comment.
One comment, but otherwise LGTM
| : undefined | ||
| } | ||
|
|
||
| async function parseSseJsonRpcResponse(response: Response): Promise<Mcp.Response | undefined> { |
There was a problem hiding this comment.
does the MCP SDK not have utils for doing this? this feels pretty fragile
There was a problem hiding this comment.
MCP SDK does not afaik... We could add a new dependency like eventsource-parser if we wanted get rid of some of this code wdyt?
| ...message, | ||
| params: { | ||
| ...message.params, | ||
| ['_meta']: { ...message.params?.['_meta'], [Mcp.credentialMetaKey]: parsed }, |
👁️ Cyclops Security Review
🧭 Audit failed · mode=
Findings
⚙️ Controls
📜 26 events🔍 |
Summary
Transport.http()from protocol handlers for Payment-Auth, x402, and MCP-over-HTTP challenges behind the default payment-aware fetch.Transport.mcp()while MCP-over-HTTP stays inside the HTTP fetch transport.Validation