feat(jetsocat): add concurrent reads support for MCP proxy #1560
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Refactor MCP proxy to support server-initiated messages (notifications, progress updates, and server -> client requests) by enabling concurrent reads from both client and server. Issue: DGW-314
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the MCP proxy implementation to support bidirectional communication and improve cancel safety. The key changes address the need for concurrent read/write operations, particularly for Process and NamedPipe transports.
- Splits the monolithic
forward_request()into separatesend_message()andread_message()methods to enable concurrent reading and writing - Implements cancel-safe message reading using internal buffers that persist across
tokio::select!cancellations - Refactors error handling to distinguish between transient and fatal errors with structured
SendErrorandReadErrortypes
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/mcp-proxy/src/lib.rs | Core refactoring: replaces forward_request() with send_message()/read_message(), adds cancel-safe buffering, updates error types, and renames transport structs |
| jetsocat/src/mcp.rs | Rewrites proxy loop to use tokio::select! for concurrent read/write operations with cancel-safe line reading |
| testsuite/tests/mcp_proxy.rs | Updates test cases to use new API (send_message/read_message), verifies new error handling behavior |
| testsuite/tests/cli/jetsocat.rs | Updates error message assertions and adjusts broken pipe test expectations |
| testsuite/src/mcp_server.rs | Adds TODO comment for unsolicited message support |
| testsuite/src/mcp_client.rs | Adds TODO comment for request ID tracking |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactor MCP proxy to support server-initiated messages (notifications, progress updates, and server -> client requests) by enabling concurrent reads from both client and server.
Issue: DGW-314