-
Notifications
You must be signed in to change notification settings - Fork 6.1k
feat: experimental codex stdio-to-uds subcommand
#5350
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| use std::io; | ||
| use std::io::Read; | ||
| use std::io::Write; | ||
| use std::net::Shutdown; | ||
| use std::os::unix::net::UnixStream; | ||
| use std::path::Path; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard stdio-to-uds crate for non-Unix builds
The new stdio-to-uds implementation imports std::os::unix::net::UnixStream unconditionally and the CLI always builds this crate. On Windows, the std::os::unix module is absent, so adding this module to the workspace will cause cargo build for the CLI to fail even if the subcommand is never used. Other platform-specific code paths in the repo use #[cfg] guards; similar gating or a stub implementation is needed here to keep non-Unix builds compiling.
Useful? React with 👍 / 👎.
c744eec to
cdd7b30
Compare
|
@codex review pls |
|
Codex Review: Didn't find any major issues. 🚀 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A README for this crate would probably be useful if you expect this to stick around
|
@gpeal good call: I adapted the PR body so it is suitable for the README |
Today, we offer two transport mechanisms for an MCP server: stdio and HTTP.
I have been experimenting with a third, which is UNIX domain socket, because it has the advantages that:
To make it easier to get started with this approach, this introduces an experimental subcommand
codex stdio-to-udsthat provides an adapter between a UDS and stdio. The idea is that someone could start an MCP server that communicates over/tmp/mcp.sock. Then the user could specify this on the fly like so:If this approach becomes popular, we could support it directly in our config:
Unfortunately, the Rust standard library does not provide support for UNIX domain sockets on Windows today even though support was added in October 2018 in Windows 10:
rust-lang/rust#56533
To address this, this PR pulls in https://crates.io/crates/uds_windows as a dependency on Windows.
stdio-to-uds/tests/stdio_to_uds.rsis a simple integration test to verify this works on all of our supported platforms.