Conversation
* origin/v1.0: stop bubbles filling screen (#495) chore: V1.0 release automation (#493) requires foreign architectures more xcompile deps x compilation tools chore: Cargo build tokenizers (#491) feat: build and release binaries to GH releases (#477) fix: width of bubbles and logging errors (#487) feat: add google provider (#489) feat: flappy goose easter egg (#479)
baxen
left a comment
baxen
left a comment
There was a problem hiding this comment.
left some comments about parallelism that i think we lost in this version
| } | ||
|
|
||
| /// Send a JSON-RPC request and wait for a response. | ||
| async fn send_message<R>(&mut self, method: &str, params: Value) -> Result<R, Error> |
| Box::pin(async move { | ||
| let transport = transport.lock().await; | ||
|
|
||
| // Initialize (start) transport on the first call. |
There was a problem hiding this comment.
nit: this might be surprising behavior, might make this more explicit by instead asserting initialization here and moving the start call to another method or maybe at instantiation?
| Ok(response) | ||
| } | ||
| JsonRpcMessage::Request(request) => { | ||
| // Serialize request & wait for response |
There was a problem hiding this comment.
I don't see how this version works, but i might be missing something?
It definitely works if everything is used in serial, a request gets sent in, the next message to receive is the response to that request, this exits happily.
But if we send multiple requests in parallel, i don't see that there's any guarantee they come back in order?
I think we need something like the previous Session implementation where each outbound request goes with an attached response_tx, so that this can await the response_rx.
| /// - receiving JSON-RPC messages as strings | ||
| /// - closing the transport cleanly | ||
| #[async_trait] | ||
| pub trait Transport: Send + 'static { |
There was a problem hiding this comment.
start/close make sense, but i think we want to have a Receiver and Sender so we can more carefully handle parallelism. there needs to be something polling these to route responses back to their originating requests, and implementing send/receive from scratch would mean we need to be more careful with that handling - i wouldn't want to have to reimplement the sleep vs close etc logic
…ntation * checks out the changes from 'kalvin/mcp-client-trait' branch Co-authored-by: kalvinnchau <kalvin@block.xyz>
|
changes transferred over to this PR - #505 |
2 participants
Provides a minimal MCP client sdk which will be used to write goose as an MCP client.
https://spec.modelcontextprotocol.io/specification/basic/
Methods we implemented: