uds: add async Unix socket crate#18254
Conversation
## Summary - add a codex-uds crate with async UnixListener and UnixStream wrappers - expose helpers for private socket directory setup and stale socket path checks - migrate codex-stdio-to-uds onto codex-uds and Tokio-based stdio/socket relaying - update the CLI stdio-to-uds command path for the async runner ## Tests - cargo test -p codex-uds -p codex-stdio-to-uds - cargo test -p codex-cli - just fmt - just fix -p codex-uds - just fix -p codex-stdio-to-uds - just fix -p codex-cli - just bazel-lock-check - git diff --check
7d60a42 to
32adbbe
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32adbbe5c1
ℹ️ 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 tokio_util::compat::Compat; | ||
| use tokio_util::compat::FuturesAsyncReadCompatExt; | ||
|
|
||
| pub(super) type Stream = Compat<Async<WindowsUnixStream>>; |
There was a problem hiding this comment.
Preserve write-half shutdown on Windows UDS streams
platform::Stream on Windows is Compat<Async<WindowsUnixStream>> (codex-rs/uds/src/lib.rs:168). UnixStream::poll_shutdown forwards to this adapter, whose shutdown path maps to poll_close; for async_io::Async, poll_close only flushes. As a result, socket_writer.shutdown().await in stdio-to-uds::run no longer sends a real write-half close on Windows, so peers waiting for EOF can hang.
Useful? React with 👍 / 👎.
viyatb-oai
left a comment
There was a problem hiding this comment.
Reviewing in the context of SSH remote control over Unix sockets.
| )); | ||
| } | ||
|
|
||
| let permissions = metadata.permissions(); |
There was a problem hiding this comment.
In the SSH-over-UDS remote-control model, the directory contract should be exactly 0700, not only "no group/other bits". As written, an existing $CODEX_HOME/app-server-control with mode 0600 passes this check even though the owner cannot traverse it. I would compare (mode & 0o777) != SOCKET_DIR_MODE and set it to 0700, maybe also add a regression test for an existing 0600 directory.
There was a problem hiding this comment.
@euroelessar seems worth capturing this in a comment with this code?
bolinfest
left a comment
There was a problem hiding this comment.
Can you check on the P1 reported by Codex?
| )); | ||
| } | ||
|
|
||
| let permissions = metadata.permissions(); |
There was a problem hiding this comment.
@euroelessar seems worth capturing this in a comment with this code?
Summary
Tests