Feature/add rust mcp client integration example#390
Conversation
|
Thanks for the great work. We’re currently upgrading our MCP client to also work with our MCP proxy (see PR #378). Can you confirm whether this change impacts your change in any way? |
Thank so much for the heads up and sorry for the late reply, I was resolving merge conflicts and wanted to understand more the proxy addition (even though it doesn't use certification persistent as the PR does) and make sure it works fine. I also made the code simpler |
There was a problem hiding this comment.
The MCP client examples are valuable and well-documented. The certificate strategy pattern is a good addition for supporting both development and production use cases.
Requested Changes
1. Please rebase to incorporate the latest changes from main
The PR has many merge commits and conflicts with recent changes.
2. Isolate changes from other PRs
This PR includes changes from multiple other PRs (388, 389, 393, 395, 378). Please rebase so only the example-related and certificate strategy changes are included. This will make the PR much easier to review and reduce merge conflicts.
3. Consider adding MCP proxy examples too
It would be great to also include examples showing how to connect to the MCP proxy, not just the direct server connection. This would provide more complete documentation for users.
Issues to Address
4. CertificateStrategy::Persistent doesn't actually load persistent certs
Self::Persistent(_path) => {
// Since we can't reconstruct the rcgen::Certificate objects from PEM,
// we'll generate a new chain (this is fine for client connections)
CertificateChain::generate() // <-- Generates NEW certs, doesn't load!
}This is misleading - the name suggests it loads from disk but it generates new certs. Either fix the loading or rename/document the behavior clearly.
5. Examples use deprecated parameter name
"no_symbols": false // Should be "include_symbols": true based on PR #377If PR #377 is merged first, these examples will break. Please update to use include_symbols.
6. Improve error handling in examples
The examples use .unwrap() in several places that could panic:
let x = content_item.as_text().unwrap().text.to_string();Consider using proper error handling with ? or if let.
Thanks for the great work on this! The documentation (README, MTLS.md) is particularly helpful.
0ceb441 to
6de4c80
Compare
6de4c80 to
d074865
Compare
|
Thanks @ahmedhesham6 for the detailed review!
please let me know for any changes! |

Description
Brief description of what this PR does.
Related Issues
Fixes #250
Changes Made
Testing
Screenshots
Running the tool
generate_passwordfrom the server using 3 different waysBreaking Changes
rmcpto version 0.9.1 to match rig's versionreqwestto the latest 0.12.26, it was rig or mcp complaining can't remember who to be honest