Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions tests/tools/test_bash.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
from __future__ import annotations

import sys
import time

import pytest

from tests.mock.utils import collect_result
Expand Down Expand Up @@ -54,6 +57,27 @@ async def test_handles_timeout(bash):
assert "Command timed out after 1s" in str(err.value)


@pytest.mark.asyncio
async def test_returns_promptly_when_detached_child_keeps_pipes_open(bash, tmp_path):
script = tmp_path / "spawn_detached.py"
script.write_text(
"import subprocess, sys\n"
"subprocess.Popen([sys.executable, '-c', 'import time; time.sleep(15)'])\n"
"print('parent done')\n",
encoding="utf-8",
)

start = time.monotonic()
result = await collect_result(
bash.run(BashArgs(command=f'"{sys.executable}" "{script}"', timeout=30))
)
elapsed = time.monotonic() - start

assert result.returncode == 0
assert "parent done" in result.stdout
assert elapsed < 10


@pytest.mark.asyncio
async def test_truncates_output_to_max_bytes(bash):
config = BashToolConfig(max_output_bytes=5)
Expand Down
54 changes: 47 additions & 7 deletions vibe/core/tools/builtins/bash.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,37 @@ def _get_shell_executable() -> str | None:
return os.environ.get("SHELL")


_PIPE_DRAIN_GRACE_SECONDS = 2.0
_PIPE_READ_CHUNK_BYTES = 65536


async def _pump_stream(
stream: asyncio.StreamReader | None, buffer: bytearray, max_bytes: int
) -> None:
# Reading continues past max_bytes (discarding) so a chatty child never
# blocks on a full pipe and can reach EOF.
if stream is None:
return
while chunk := await stream.read(_PIPE_READ_CHUNK_BYTES):
if len(buffer) < max_bytes:
buffer.extend(chunk[: max_bytes - len(buffer)])


async def _wait_for_returncode(proc: asyncio.subprocess.Process, timeout: int) -> None:
# Process.wait() only resolves once every pipe has hit EOF
# (BaseSubprocessTransport._try_finish), so a detached child inheriting
# the pipes stalls it long after the command itself exited. returncode is
# set as soon as the process exits, so poll it instead.
loop = asyncio.get_running_loop()
deadline = loop.time() + timeout
delay = 0.01
while proc.returncode is None:
if loop.time() >= deadline:
raise TimeoutError
await asyncio.sleep(delay)
delay = min(delay * 2, 0.25)


def _get_base_env() -> dict[str, str]:
base_env = {**os.environ, "CI": "true", "NONINTERACTIVE": "1", "NO_TTY": "1"}

Expand Down Expand Up @@ -530,22 +561,31 @@ async def run(
**kwargs,
)

stdout_buf = bytearray()
stderr_buf = bytearray()
pumps = [
asyncio.create_task(_pump_stream(proc.stdout, stdout_buf, max_bytes)),
asyncio.create_task(_pump_stream(proc.stderr, stderr_buf, max_bytes)),
]
try:
stdout_bytes, stderr_bytes = await asyncio.wait_for(
proc.communicate(), timeout=timeout
)
await _wait_for_returncode(proc, timeout)
except TimeoutError:
await kill_async_subprocess(proc)
raise self._build_timeout_error(args.command, timeout)
finally:
await asyncio.wait(pumps, timeout=_PIPE_DRAIN_GRACE_SECONDS)
for pump in pumps:
pump.cancel()
await asyncio.gather(*pumps, return_exceptions=True)

stdout = (
decode_safe(stdout_bytes, from_subprocess=True).text[:max_bytes]
if stdout_bytes
decode_safe(bytes(stdout_buf), from_subprocess=True).text[:max_bytes]
if stdout_buf
else ""
)
stderr = (
decode_safe(stderr_bytes, from_subprocess=True).text[:max_bytes]
if stderr_bytes
decode_safe(bytes(stderr_buf), from_subprocess=True).text[:max_bytes]
if stderr_buf
else ""
)

Expand Down
7 changes: 6 additions & 1 deletion vibe/core/utils/async_subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,11 @@ async def kill_async_subprocess(
exc_info=True,
)

await proc.wait()
# Process.wait() only resolves once every pipe has hit EOF
# (BaseSubprocessTransport._try_finish); a detached grandchild holding
# an inherited pipe would stall it forever even though the child was
# just killed. returncode is set on process exit, so poll it instead.
while proc.returncode is None:
await asyncio.sleep(0.02)
except (ProcessLookupError, PermissionError, OSError):
pass