[2/8] Support piped stdin in exec process API#18086
Conversation
aibrahim-oai
force-pushed
the
dev/remote-mcp-env-config
branch
from
e776798 to
d91856d
Compare
aibrahim-oai
force-pushed
the
dev/remote-mcp-exec-stdin
branch
from
315ecdf to
4942819
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 315ecdfb2c
ℹ️ 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".
| /// stdin see EOF immediately. Remote MCP stdio uses `Piped` because rmcp | ||
| /// must write JSON-RPC request bytes to the child process stdin after the | ||
| /// process has started. | ||
| pub stdin: ExecStdinMode, |
There was a problem hiding this comment.
Make stdin optional at the wire level. As added, ExecParams now requires stdin, so older process/start clients that omit it fail deserialization with invalid params instead of starting a process. There are existing JSON callers that don't send this field (for example in websocket process tests), so this change introduces a protocol break. Add a serde default of Closed.
Useful? React with 👍 / 👎.
aibrahim-oai
force-pushed
the
dev/remote-mcp-env-config
branch
from
d91856d to
fbdc10d
Compare
aibrahim-oai
force-pushed
the
dev/remote-mcp-exec-stdin
branch
2 times, most recently
from
cd30150 to
8978634
Compare
aibrahim-oai
force-pushed
the
dev/remote-mcp-env-config
branch
from
fbdc10d to
696aaba
Compare
## Summary - Add an MCP server environment setting with local as the default. - Thread the default through config serialization, schema generation, and existing config fixtures. ## Stack ```text o #18027 [8/8] Fail exec client operations after disconnect │ o #18025 [7/8] Cover MCP stdio tests with executor placement │ o #18089 [6/8] Wire remote MCP stdio through executor │ o #18088 [5/8] Add executor process transport for MCP stdio │ o #18087 [4/8] Abstract MCP stdio server launching │ o #18020 [3/8] Add pushed exec process events │ o #18086 [2/8] Support piped stdin in exec process API │ @ #18085 [1/8] Add MCP server environment config │ o main ``` Co-authored-by: Codex <noreply@openai.com>
aibrahim-oai
force-pushed
the
dev/remote-mcp-exec-stdin
branch
2 times, most recently
from
1c5a436 to
f99255d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f99255d45c
ℹ️ 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".
| pub env: HashMap<String, String>, | ||
| pub tty: bool, | ||
| /// Keep non-tty stdin writable through `process/write`. | ||
| pub pipe_stdin: bool, |
There was a problem hiding this comment.
Make pipe_stdin backward-compatible at the wire level. As a required bool without #[serde(default)], older process/start clients that omit pipeStdin now fail deserialization (invalid params) instead of starting a process. This is a protocol break for existing JSON-RPC callers; defaulting to false preserves prior behavior.
Useful? React with 👍 / 👎.
aibrahim-oai
force-pushed
the
dev/remote-mcp-exec-stdin
branch
2 times, most recently
from
893c39b to
ed6b28c
Compare
Add pipe_stdin to process/start so non-tty processes can opt into a writable stdin pipe for process/write. Co-authored-by: Codex <noreply@openai.com>
aibrahim-oai
force-pushed
the
dev/remote-mcp-exec-stdin
branch
from
ed6b28c to
086ae0a
Compare
1 participant
Summary
Stack