Skip to content

Commit c2ac766

Browse files
committed
fix(shell): stop blocking until timeout when a detached child holds the pipes
The foreground shell path waited for stdout/stderr EOF before checking the exit code, so a detached child that inherited the pipes kept the tool blocked for the full command timeout, after which the run was wrongly reported as killed by timeout. Process exit is now observed by polling returncode (asyncio gates its exit waiters on pipe disconnection), and remaining output is drained for a bounded grace period after the shell exits.
1 parent 4a550ef commit c2ac766

3 files changed

Lines changed: 65 additions & 9 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ Only write entries that are worth mentioning to users.
1111

1212
## Unreleased
1313

14+
- Shell: Fix the shell tool blocking until the full command timeout when a detached child process inherits stdout/stderr, then wrongly reporting a timeout kill. The tool now returns shortly after the shell itself exits and drains remaining pipe output for a bounded grace period
15+
1416
## 1.49.0 (2026-07-16)
1517

1618
**Highlights**: The completion-token budget for Kimi providers now adapts to the model's remaining context window, reducing context-length overflow errors on long turns

src/kimi_cli/tools/shell/__init__.py

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import asyncio
2+
import contextlib
23
from collections.abc import Callable
34
from pathlib import Path
45
from typing import Self, override
@@ -21,6 +22,18 @@
2122

2223
MAX_FOREGROUND_TIMEOUT = 5 * 60
2324
MAX_BACKGROUND_TIMEOUT = 24 * 60 * 60
25+
PIPE_DRAIN_GRACE = 2.0
26+
"""Seconds to keep draining stdout/stderr after the shell process exits.
27+
28+
A detached child that inherited the pipes can keep them open long after the
29+
shell itself has exited; without a bound the tool would block until the full
30+
command timeout waiting for an EOF that may never come."""
31+
EXIT_POLL_INTERVAL = 0.05
32+
"""Seconds between exit checks while the shell process is running.
33+
34+
`KaosProcess.wait()` may not resolve until all pipes close (asyncio gates its
35+
exit waiters on pipe disconnection), so process exit is observed by polling
36+
`returncode` instead."""
2437

2538

2639
class Params(BaseModel):
@@ -244,21 +257,39 @@ async def _read_stream(stream: AsyncReadable, cb: Callable[[bytes], None]):
244257
# EOF instead of hanging forever waiting for input that will never come.
245258
process.stdin.close()
246259

260+
async def _wait_exit() -> int:
261+
while (exitcode := process.returncode) is None:
262+
await asyncio.sleep(EXIT_POLL_INTERVAL)
263+
return exitcode
264+
265+
def _consume_exception(task: asyncio.Future[tuple[None, None]]) -> None:
266+
# When the read task is abandoned after cancel (user interrupt or
267+
# command timeout), retrieve its outcome so the event loop does
268+
# not report "exception was never retrieved".
269+
if not task.cancelled():
270+
task.exception()
271+
272+
read_task = asyncio.gather(
273+
_read_stream(process.stdout, stdout_cb),
274+
_read_stream(process.stderr, stderr_cb),
275+
)
276+
read_task.add_done_callback(_consume_exception)
247277
try:
248-
await asyncio.wait_for(
249-
asyncio.gather(
250-
_read_stream(process.stdout, stdout_cb),
251-
_read_stream(process.stderr, stderr_cb),
252-
),
253-
timeout,
254-
)
255-
return await process.wait()
278+
exitcode = await asyncio.wait_for(_wait_exit(), timeout)
256279
except asyncio.CancelledError:
280+
read_task.cancel()
257281
await process.kill()
258282
raise
259283
except TimeoutError:
284+
read_task.cancel()
260285
await process.kill()
261286
raise
287+
# The shell has exited, but a detached child that inherited the
288+
# pipes can keep them open indefinitely; drain what is left
289+
# instead of waiting for an EOF that may never come.
290+
with contextlib.suppress(TimeoutError):
291+
await asyncio.wait_for(read_task, PIPE_DRAIN_GRACE)
292+
return exitcode
262293

263294
def _shell_args(self, command: str) -> tuple[str, ...]:
264295
return (str(self._shell_path), "-c", command)

tests/tools/test_shell_bash.py

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import asyncio
66
import platform
7+
import time
78

89
import pytest
910
from inline_snapshot import snapshot
@@ -97,6 +98,23 @@ async def test_command_timeout_expires(shell_tool: Shell):
9798
assert result.brief == snapshot("Killed by timeout (1s)")
9899

99100

101+
async def test_detached_child_holding_pipes_does_not_block_until_timeout(shell_tool: Shell):
102+
"""A detached child that inherits stdout/stderr must not stall the tool.
103+
104+
The shell exits immediately, but the backgrounded sleep keeps the pipes
105+
open; the tool should return shortly after the shell exits instead of
106+
blocking until the command timeout waiting for pipe EOF.
107+
"""
108+
start = time.monotonic()
109+
result = await shell_tool(Params(command="sleep 30 & echo started", timeout=25))
110+
elapsed = time.monotonic() - start
111+
assert not result.is_error
112+
assert "started" in result.output
113+
# Shell exit plus PIPE_DRAIN_GRACE, with slack for slow CI; the old
114+
# behavior would block for the full 25s timeout and report an error.
115+
assert elapsed < 20
116+
117+
100118
async def test_environment_variables(shell_tool: Shell):
101119
"""Test setting and using environment variables."""
102120
result = await shell_tool(Params(command="export TEST_VAR='test_value' && echo $TEST_VAR"))
@@ -249,6 +267,7 @@ class _FakeProc:
249267
stdin = _NullStdin()
250268
stdout = _EmptyStream()
251269
stderr = _EmptyStream()
270+
returncode: int | None = 0
252271

253272
async def wait(self) -> int:
254273
return 0
@@ -375,12 +394,16 @@ def __init__(self) -> None:
375394
self.stdout = BlockingReadable()
376395
self.stderr = BlockingReadable()
377396
self.kill_calls = 0
397+
self.returncode: int | None = None
378398

379399
async def wait(self) -> int:
380-
return 0
400+
while self.returncode is None:
401+
await asyncio.sleep(0.01)
402+
return self.returncode
381403

382404
async def kill(self) -> None:
383405
self.kill_calls += 1
406+
self.returncode = -9
384407

385408
fake_process = FakeProcess()
386409

0 commit comments

Comments
 (0)