mcp: introduce Requests

[jba]

Handlers and client/server methods take a single request argument combining the session and params. This slightly simplifies the signature for handlers, since there is no longer an often-ignored session argument.

But more importantly, it opens the door to adding more information in requests, such as auth info and HTTP request headers.

For #243.

[findleyr]

Just a thought, but would it be cleaner to have ClientRequest and ServerRequest?

Not only will it be slightly shorter, but there may be fields on request that only relate to the client, or server.

[jba]

This is the type that is used in the middleware system. To split into two types would mean splitting middleware.
Current state:

type MethodHandler[S Session] func(ctx context.Context, method string, req *Request[S]) (result Result, err error)
type Middleware[S Session] func(MethodHandler[S]) MethodHandler[S] 
func (c *Client) AddSendingMiddleware(middleware ...Middleware[*ClientSession])
func (c *Client) AddReceivingMiddleware(middleware ...Middleware[*ClientSession]) 
func (s *Server) AddSendingMiddleware(middleware ...Middleware[*ServerSession])
func (s *Server) AddReceivingMiddleware(middleware ...Middleware[*ServerSession])

You're asking to double that.

[jba]

Oh, wait, you're not actually splitting all of it, just MethodHandler and Middleware. That still seems like a lot. But we could land this first, then I'll try that?

[jba]

I'll try splitting RequestFor. That will be what most people will see anyway; not many people will write middleware.

[jba]

Latest commit splits RequestFor into two parts. Leaving Request as it is.

[jba]

I like the separate Client and Server requests.

[findleyr]

Ok, here's the problem that I see: eventually, we will want to encode information such as auth headers in the request, which means that it will be the responsibility of ServerSession.handle to map this information into the handler. But that means that ServerSession.handle will need to pack everything into a ServerRequest, and that needs to be transmitted through the method handler. But the method handler does not handle a ServerRequest, it handles a Request[*ServerSession]. In other words, you're necessarily piping this information through the Request type, which would lose any specialization for servers.

Therefore, I think we need to either make the entire stack of ServerXXX objects, or revert back to RequestFor[Session, Params], or make Request an interface.

[findleyr]

One advantage of simply having Request be an interface is that users can reuse middleware for both the client and server, without needing a wrapper.

We can remove the *ClientSession | *ServerSession from the session interface, so that it's an ordinary interface, and add an accessor. Note that Session is already closed since it has unexported methods, so we don't need to explicitly mark it as a union.

[findleyr]

Isn't this transport specific metadata? Do we want to hard-code it in this way?
It's also server-specific, and doesn't make sense for the client.

I think it should be an arbitrary annotation API.

[jba]

An "arbitrary annotation API" is a map[string]any. You give up compile-time name and type checking. And you also give up the ability to read the godoc and know what might be there and what definitely isn't there.

I'd rather have fields that are just empty if unused. The header is only non-nil on servers that use an HTTP transport. That's fine, users will just check. That's still better than a grab-bag IMO.

[jba]

This is partly obsolete: with separate client and server requests, HTTP headers will not appear in client requests.

[findleyr]

Sorry this is so difficult. Perhaps we should meet to discuss.

[findleyr]

Ok, here's the problem that I see: eventually, we will want to encode information such as auth headers in the request, which means that it will be the responsibility of ServerSession.handle to map this information into the handler. But that means that ServerSession.handle will need to pack everything into a ServerRequest, and that needs to be transmitted through the method handler. But the method handler does not handle a ServerRequest, it handles a Request[*ServerSession]. In other words, you're necessarily piping this information through the Request type, which would lose any specialization for servers.

Therefore, I think we need to either make the entire stack of ServerXXX objects, or revert back to RequestFor[Session, Params], or make Request an interface.

[findleyr]

LGTM, thanks!

[findleyr]

These seem like they could be one type with a third type parameter.

[jba]

I think there is a lot of cleanup of unexported code I could do. Tomorrow I'll follow up on this and others.