mcp: HTTP Header Standardization for x-mcp-header#915
mcp: HTTP Header Standardization for x-mcp-header#915guglielmo-san wants to merge 24 commits intomainfrom
Conversation
…ard request headers
…treamableClientConn
…rs for tool calls
…e comparison for parameter validation
…es and add structural validation
…ttp_standardization_2 # Conflicts: # mcp/streamable.go # mcp/streamable_client_test.go # mcp/streamable_headers.go # mcp/streamable_headers_test.go # mcp/streamable_test.go
…ernal JSON unmarshaling
…ors in ClientSession
…treamable_test.go
…_http_standardization_2' into guglielmoc/SEP-2243_http_standardization_2
guglielmo-san
changed the title
| // Avoid sending nil over the wire. | ||
| params.Arguments = map[string]any{} | ||
| } | ||
| if tool := cs.getCachedTool(params.Name); tool != nil { |
There was a problem hiding this comment.
Please also think how modelcontextprotocol/modelcontextprotocol#2549 will factor in into this solution.
|
|
||
| bodyVal := unmarshalPrimitive(argRaw) | ||
| if bodyVal == nil { | ||
| continue |
There was a problem hiding this comment.
IIUC this is a situation where the header is present, but the value in arguments is not a primitive. Please add a comment explaining why it is ok to ignore this.
There was a problem hiding this comment.
Actually it would probably be better to return an error. The SEP does not explicitly define this case but it makes more sense to me.
There was a problem hiding this comment.
Maybe we should also validate the schema on AddTool as well to make a situation where a server serves incorrect tool fail early?
| if err != nil { | ||
| return nil, err | ||
| } | ||
| result.Tools = filterValidTools(result.Tools) |
There was a problem hiding this comment.
In general this proposal adds HTTP specific functionality, so I'm a bit surprised that the spec requires general filtering. IMO ideally this change should be contained within streamable transport as much as possible. I'll ask on the SEP for clarification.
maciej-kisiel
left a comment
maciej-kisiel
left a comment
There was a problem hiding this comment.
Also some additional comments from AI review :)
…er validation to support JSON schema annotations
| return | ||
| } | ||
| transport.connection.toolLookup = func(name string) *Tool { | ||
| server.mu.Lock() |
There was a problem hiding this comment.
Using the mutex from a different file is probably not a good practice. Consider exposing a private method on the server.
2 participants
Description
Implements SEP-2243 (HTTP Header Standardization) for x-mcp-param custom header.
Fixes #905