fix(shell): stop blocking until timeout when a detached child holds the pipes

[ayaangazali]

Related Issue

Resolve #2468

Description

In the foreground shell path, _run_shell_command waits for stdout/stderr EOF (asyncio.wait_for(gather(read stdout, read stderr), timeout)) before ever checking the exit code. A command like some_daemon & echo done leaves a detached child holding the write ends of the pipes, so EOF never arrives: the tool blocks for the entire command timeout even though the shell exited immediately, then reports "Command killed by timeout" for a command that actually succeeded. That is the hang in #2468.

Two things I learned while fixing it, which shaped the implementation:

1. Waiting on process.wait() instead does not work: asyncio gates its subprocess exit waiters on pipe disconnection, so wait() blocks exactly the same way (verified empirically on macOS and Linux Python 3.14: wait() returned only when the detached child died, while returncode was set within 50ms of the shell exiting). So exit is observed by polling returncode at 50ms intervals, which the KaosProcess protocol guarantees on every backend.
2. Abandoning the reader gather after cancel triggers asyncio's "_GatheringFuture exception was never retrieved" loop-level error, which prompt_toolkit surfaces as a full-screen "Unhandled exception in event loop" banner that freezes the TUI. The existing PTY e2e tests (test_ctrl_c_during_running_turn_interrupts, test_shell_cancel_running_command_kills_process_and_recovers) caught this in my first attempt, hence the _consume_exception done callback.

Behavior after the fix:

• Shell exits while something still holds the pipes: remaining output is drained for a bounded PIPE_DRAIN_GRACE (2s), then the tool returns the real exit code. Output produced by a detached child after that grace is dropped, which I think is the right semantic: the shell finishing is what "the command finished" means.
• Genuinely long-running commands: unchanged, still killed at the timeout with the same error message.
• Ctrl-C cancel: unchanged (kill + raise), minus the event loop leak.
• Normal fast commands: unchanged output, worst case ~50ms added exit-detection latency.

Note this intentionally does not implement full child-process tracking/reaping that the issue mentions as the ideal. The detached child keeps running (same as before the fix); the change just stops it from stalling the agent loop. Figured the bigger cleanup story needs maintainer direction first.

Tests: new test_detached_child_holding_pipes_does_not_block_until_timeout (fails at 25s timeout before the fix, returns in ~2s after). The fake processes in existing tests grew a returncode attribute since the code now polls it, and the cancel-test fake now models "running until killed" to keep testing the same scenario it did before.

Checklist

• I have read the CONTRIBUTING document.
• I have linked the related issue, if any.
• I have added tests that prove my fix is effective or that my feature works.
• I have run make gen-changelog to update the changelog. (hand-edited CHANGELOG.md in the same style, i don't have the Kimi API setup the skill needs)
• I have run make gen-docs to update the user documentation. (no user-facing docs describe the shell tool's pipe/timeout behavior, nothing to regenerate)

---

being upfront: freshman here, i dug through this one with claude code as a rubber duck for the asyncio semantics (the wait() thing surprised both of us). tested everything i could locally but if the approach is off i'm all ears, learning as i go :)

---

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[devin-ai-integration]

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.