Refactor run() to accept generic async streams - #8
nikomatsakis wants to merge 1 commit into
Conversation
nikomatsakis
commented
Nov 10, 2025
nikomatsakis
commented
- Add run_with_streams<R, W>() method accepting generic AsyncRead/AsyncWrite streams
- Keep existing run() method for backward compatibility, delegating to run_with_streams()
- Enables library usage with custom streams (duplex, Unix sockets, etc.) beyond stdio
- Add run_with_streams<R, W>() method accepting generic AsyncRead/AsyncWrite streams - Keep existing run() method for backward compatibility, delegating to run_with_streams() - Enables library usage with custom streams (duplex, Unix sockets, etc.) beyond stdio Co-authored-by: Claude <claude@anthropic.com>
|
Hi Niko, I'm sorry I never saw this but for some reason GitHub hasn't been sending me notifications about PRs on the repo. 🤦♂️ I'll look soon. |
zeenix
left a comment
zeenix
left a comment
There was a problem hiding this comment.
🤖 Chatgpt GPT-5.6 on behalf of Zeeshan: Apologies this sat unreviewed: notifications for this repository were accidentally disabled, so it wasn’t seen until now.
The generic stream abstraction itself is sound, but the branch conflicts directly with the shutdown work just merged in #23. Please rebase and add the custom-stream regression test below; resolving the conflict by retaining this branch’s old loop would reintroduce the shutdown bug. CONTRIBUTING.md also asks for a package prefix in the commit subject (for example, ♻️ mcp: Accept generic async streams). If this lands, #18 must preserve this public entry point during its server rewrite.
| self.run_with_streams(stdin, stdout).await | ||
| } | ||
|
|
||
| pub async fn run_with_streams<R, W>(&mut self, reader: R, writer: W) -> Result<()> |
There was a problem hiding this comment.
🤖 Chatgpt GPT-5.6 on behalf of Zeeshan: [blocking] Please rebase this onto current main and reapply the generic reader/writer abstraction around the newly merged shutdown loop. The body here still contains the pre-#23 Arc<Mutex<bool>> polling implementation; resolving the conflict in its favor would discard the current signal racing, error propagation, and bounded cleanup behavior. Please also add a tokio::io::duplex regression test that exercises run_with_streams.
2 participants
