fix: run_command() timeout via reader thread#6
Conversation
readline() blocked indefinitely on hung processes, making timeout_seconds useless. Now a daemon reader thread handles I/O while the main thread enforces a monotonic deadline. Process is terminated/killed when the deadline expires. Partial output is captured before timeout. +6 tests covering output capture, exit codes, log writing, timeout kill, partial output, and normal completion with timeout set.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a410ac325
ℹ️ 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".
| # Main thread enforces the timeout — reader thread cannot block us. | ||
| deadline = time.monotonic() + timeout_seconds if timeout_seconds else None | ||
| timed_out = False | ||
| while reader.is_alive(): |
There was a problem hiding this comment.
Keep enforcing timeout after reader thread exits
The timeout logic is gated by while reader.is_alive(), so if a child process closes or redirects both stdout/stderr early (e.g., daemonizes to /dev/null) the reader thread exits immediately even though the process is still running. In that case this path falls through to process.wait() with no timeout and can block indefinitely, which reintroduces a hang despite timeout_seconds being set. Timeout enforcement should be independent of reader-thread liveness.
Useful? React with 👍 / 👎.
Summary
readline()loop with a daemon reader threadthread.join()with monotonic deadlineTest plan
python3 -m nightshift run --agent claudeagainst a repo and interrupt