fix: close stdin on streaming Port paths#38
Merged
joshrotenberg merged 1 commit intomainfrom Apr 10, 2026
Merged
Conversation
2 tasks
The non-streaming execute path was fixed in #34, but the streaming paths in Exec.stream/2 and ExecResume.stream/2 still opened a raw Port with stdin inherited from the parent. Codex CLI 0.118+ reads stdin at startup and blocks forever on the open-but-empty pipe, causing streams to hang until the internal 300s safety timeout. Extract the shell-wrapping logic from Command.run_with_closed_stdin into a shared Command.shell_cmd_args/3 helper that both execute and streaming paths now use. Streaming opts out of the 2>&1 stderr merge so stderr flows to the parent without contaminating NDJSON on stdout. Verified against codex-cli 0.119.0: Exec.stream/2 now completes in ~13s with the expected thread.started / turn.started / item.completed / turn.completed events instead of hanging on the safety timeout. Closes #37
84f9582 to
240ebaf
Compare
This was referenced Apr 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Exec.stream/2,ExecResume.stream/2) hanging indefinitely againstcodex-cli ≥ 0.118because the Port inherited stdin from the parent and Codex reads stdin at startup.Command.run_with_closed_stdininto a sharedCommand.shell_cmd_args/3helper so execute and streaming paths share one implementation.2>&1stderr merge that the execute path uses, so stderr flows to the parent's stderr without contaminating NDJSON on stdout.Closes #37. Complements #34 which fixed the same root cause in
command.ex(the non-streamingexecutepath) but didn't touch the streaming paths.Test plan
mix test— 217 tests pass, no regressionscodex-cli 0.119.0:after 300_000safety timeout fired with zero events.Reading additional input from stdin...message from Codex no longer appears, confirming stdin is actually closed.Background
Discovered while building a
GenAgentbackend adapter on top of this package. The GenAgent Codex backend currently uses the non-streamingexec_jsonpath (which was already fixed by #34), so this fix doesn't unblock anything immediately for that consumer — but any future streaming consumer would have hit the same wall. Getting the streaming path into the same state as execute makes the package's surface consistent.There is a sibling issue on
claude_wrapper_ex(#42) with the same latent root cause — the Claude CLI doesn't currently trigger it, but the code path is structurally identical. Fix PR to follow there.