Skip to content
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

feat: MCP client sdk matching pending requests #505

Merged
merged 23 commits into from
Dec 20, 2024

Conversation

salman1993
Copy link
Collaborator

@salman1993 salman1993 commented Dec 20, 2024

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:

  • initialize
  • list resources
  • read resource
  • list tools
  • call tool

salman1993 and others added 18 commits December 16, 2024 23:26
* 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)
…ntation

* checks out the changes from 'kalvin/mcp-client-trait' branch

Co-authored-by: kalvinnchau <[email protected]>
Copy link
Collaborator

@baxen baxen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Collaborator

@kalvinnchau kalvinnchau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

some potential places for better ergonomics (type state, error handling) but can also be handled in another PR

Comment on lines +36 to +43
struct TransportServiceInner<T: Transport> {
transport: Arc<T>,
router: Mutex<Option<MessageRouter>>,
initialized: AtomicBool,
}

impl<T: Transport> TransportServiceInner<T> {
async fn ensure_initialized(&self) -> Result<MessageRouter, ServiceError> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be worth exploring the use of the Type State Pattern here? We could get compile time guarantees about the state of the Transport and have no need for the ensure_initialized further down in TransportService::call's method. (Disclaimer: i only recently learned about this pattern https://cliffle.com/blog/rust-typestate/)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would probably be possible to also use the Type State Pattern on the McpClient itself for more compile time guarantees

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this makes a lot of sense. i will defer this to another PR for now

crates/mcp-client/src/transport/sse.rs Outdated Show resolved Hide resolved
crates/mcp-client/src/transport/stdio.rs Outdated Show resolved Hide resolved
@salman1993 salman1993 merged commit 64969ba into v1.0 Dec 20, 2024
3 checks passed
lifeizhou-ap added a commit that referenced this pull request Dec 20, 2024
* v1.0:
  feat: MCP client sdk (#505)
michaelneale added a commit that referenced this pull request Dec 21, 2024
* v1.0:
  conditionally load memory system in goose server (#508)
  Adds 'instructions' field to InitializeResult (#511)
  feat: MCP client sdk (#505)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants