feat(doctor): add opt-in piped stdin for streaming fix execution - #921
feat(doctor): add opt-in piped stdin for streaming fix execution#921matt2e wants to merge 1 commit into
Conversation
Fix commands run through the streaming executor always inherited the host process's stdin. In a GUI host that stdin is never writable, so an interactive fix like `claude-agent-acp --cli auth login` — which prints an OAuth URL and then blocks reading the auth code — hangs forever (block/berd#99). Add an opt-in pipe the host can feed: - New public types `FixStdin` / `FixStdinWriter`: `FixStdin::pipe()` returns a cloneable line writer (`send_line`, which appends `\n` and flushes) plus the `FixStdin` to place in the options. Lines sent before spawn are buffered; dropping every writer delivers EOF. - `ExecuteFixOptions` gains `pub stdin: Option<FixStdin>` and a `with_stdin` builder; the existing `Debug`/`Clone`/`Default` derives are preserved via an `Arc<Mutex<Option<Receiver>>>` around the non-cloneable channel receiver. - The option threads through `execute_fix_streaming_with_env_options` → `run_command_streaming` → `run_command_streaming_blocking`, which only then sets `Stdio::piped()` on stdin and feeds the child from a detached writer thread. The thread is deliberately never joined — a writer outliving the child would park it in `rx.iter()` and hang the fix; it exits on channel close or on the post-exit EPIPE write error. - `stdin: None` keeps today's inherited-stdin behavior byte-for-byte, so terminal hosts with legitimately interactive fixes are untouched. Tests: cat echo round-trip (write + EOF + pre-spawn buffering), `read -r` prompt shape (the paste-an-auth-code flow), and no-hang-when-writer-outlives-child including post-exit `send_line` erroring instead of panicking. Existing streaming tests cover the default path. Verified with `cargo test` in crates/doctor (118 passed) plus `cargo fmt` and `cargo clippy --all-targets`. Signed-off-by: Matt Toohey <contact@matttoohey.com>
![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: d01aaa7818
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| std::thread::spawn(move || { | ||
| for line in stdin_rx.iter() { |
There was a problem hiding this comment.
When the subprocess exits while the caller retains a FixStdinWriter, this detached thread remains blocked in stdin_rx.iter() and therefore keeps the receiver open. A subsequent send_line after the execution future has completed returns Ok; only that queued write discovers EPIPE and eventually closes the channel, contrary to the public promise that sending after completion returns Err. It also leaves one parked OS thread per completed fix until the writer is dropped or another line is sent. Coordinate child completion with this worker so the receiver is closed as soon as the process exits.
Useful? React with 👍 / 👎.
1 participant
Summary
Adds opt-in piped stdin to the doctor crate's streaming fix execution, so a host can feed input (e.g. pasting an auth code into a login flow) to a running fix subprocess.
Changes
FixStdin::pipe()returns a connected(FixStdinWriter, FixStdin)pair. The caller keeps the cloneable writer and puts theFixStdininExecuteFixOptions::stdin(or uses.with_stdin(..)).FixStdinWriter::send_linequeues a line (trailing newline appended, pipe flushed) and returnsErronce the fix's stdin is closed. Lines sent before the child spawns are buffered and delivered on spawn; dropping every writer clone closes the child's stdin (EOF).stdin: Nonethe child keeps inheriting the host process's stdin, so interactive fixes in terminal hosts are unchanged.execute_fix_optionsfor the new field; its fixes are non-interactive, so they keep inherited stdin.Testing
Three new tests cover the round trip through
cat(including pre-spawn buffering and EOF-on-drop), the prompt-styleread -rshape fed while the fix runs, and the no-hang path when a writer outlives the child. Fulljust app staged ci(739 Rust + 669 frontend tests) and the crates fmt/lint/test suites pass.