Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4cfc116d7f
ℹ️ 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".
| ZSH_EXEC_BRIDGE_WRAPPER_SOCKET_ENV_VAR, | ||
| wrapper_socket_path.to_string_lossy().to_string(), | ||
| ); | ||
| cmd.env(ZSH_EXEC_BRIDGE_SOCKET_ENV_VAR, fd); |
There was a problem hiding this comment.
Isolate wrapper IPC instead of reusing one stream socket FD
This stores a single inherited socket FD in the shell environment for the entire zsh session, so every EXEC_WRAPPER invocation in that process tree talks over the same AsyncSocket stream. When commands trigger concurrent execve calls (for example pipelines or backgrounded subcommands), multiple wrapper processes can race on one stream endpoint: frames/responses can be consumed by the wrong process or interleaved, leading to request_id mismatches and spurious command failures. The previous per-connection Unix listener model avoided cross-process response mix-ups.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Aren't requests sent as framed messages to ensure they do not overlap? Do you have a recommended encoding scheme for messages to ensure they are handled by the correct process?
|
Abandoning in favor of #12584 |
No description provided.