fix(sdk/python): bound the harness subprocess exit wait — silent reasoner wedge (#778)

A reasoner calling a CLI harness provider could wedge silently forever:
stuck 'running' with no child process, no logs, and a healthy event loop.
Root-caused from a production swe-planner run that sat 26 minutes in that
state: run_cli's finally block ended with a bare `await proc.wait()`, which
parks indefinitely in two real situations —

1. Windows proactor loop loses the RegisterWaitWithQueue completion that
   resolves proc.wait() even though the child already exited (CPython
   gh-81562 / gh-111604). Both pipes are at EOF, no process remains, and
   nothing ever wakes the coroutine.
2. An orphaned grandchild outlives a killed parent while holding the
   inherited output pipes: asyncio resolves proc.wait() only after every
   pipe disconnects, and on Windows proc.kill() cannot reach grandchildren
   (pre-fix, test_run_cli_idle_watchdog_aborts_stalled_child hangs >100s
   on a real Windows machine because of exactly this).

Fix:
- _wait_process_exit(): bound the exit wait with a grace period; on
  expiry, kill whatever remains and recover the real exit status from
  proc.returncode or by polling the underlying Popen object, which needs
  no event delivery.
- _kill_group(): use `taskkill /F /T` on Windows as the killpg analog so
  kills reach the whole tree (also guards os.killpg with hasattr — it
  does not exist on Windows).
- run_cli now also kills the child when the wait loop exits abnormally
  (cancellation included); previously a cancelled run_cli left the CLI
  subprocess running and awaited its natural exit.

Tests: unit regressions for the lost-notification recovery and
kill-on-cancellation, plus a real-subprocess regression where a grandchild
holds the pipes — run_cli must conclude in bounded time on every platform.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: AbirAbbas

sdk/python/agentfield/harness/_cli.py

@@ -7,6 +7,7 @@
 import os
 import re
 import signal
+import subprocess
 from typing import Any, Dict, List, Optional, Tuple
 
 from agentfield.openrouter_attribution import apply_subprocess_env
@@ -61,6 +62,53 @@ async def _drain(
         last_activity[0] = asyncio.get_event_loop().time()
 
 
+# Grace period for the child's exit notification after its pipes hit EOF (or
+# after it was killed). Post-EOF a child is dead or exiting, so this only
+# delays the pathological cases handled in _wait_process_exit.
+_PROC_EXIT_GRACE_SECONDS = 10.0
+
+
+async def _wait_process_exit(
+    proc: asyncio.subprocess.Process, kill_group
+) -> Optional[int]:
+    """Await the child's exit without trusting the exit notification.
+
+    A bare ``await proc.wait()`` can park forever on Windows' proactor loop:
+    the RegisterWaitWithQueue completion that resolves it is occasionally lost
+    even though the child already exited (CPython gh-81562 / gh-111604 —
+    observed in production as a swe-planner reasoner silently wedged for 26
+    minutes with no child process and a perfectly healthy event loop; the
+    coroutine sat here, after both pipes had hit EOF, waiting on an exit
+    notification that never came). It also parks when an orphaned grandchild
+    outlives a killed parent while holding the output pipes, since asyncio
+    resolves ``wait()`` only once every pipe disconnects. Bound the wait; on
+    timeout, kill whatever may remain and recover the real exit status by
+    polling the underlying Popen object, which needs no event delivery.
+    Returns the exit code, or None if it cannot be determined.
+    """
+    try:
+        return await asyncio.wait_for(proc.wait(), timeout=_PROC_EXIT_GRACE_SECONDS)
+    except (asyncio.TimeoutError, TimeoutError):
+        pass
+    # The notification may have landed while we grace-waited (in which case
+    # the transport finalizer may already have dropped its Popen reference —
+    # but ``proc.returncode`` stays available), so check it at every step.
+    if proc.returncode is not None:
+        return proc.returncode
+    kill_group()
+    popen = getattr(getattr(proc, "_transport", None), "_proc", None)
+    for _ in range(100):  # <= 10s; TerminateProcess/SIGKILL act promptly
+        if proc.returncode is not None:
+            return proc.returncode
+        if popen is None:
+            break
+        returncode = popen.poll()
+        if returncode is not None:
+            return returncode
+        await asyncio.sleep(0.1)
+    return proc.returncode
+
+
 async def run_cli(
     cmd: List[str],
     *,
@@ -106,19 +154,37 @@ async def run_cli(
 
     def _kill_group() -> None:
         pid = proc.pid
-        if isinstance(pid, int) and pid > 0:
+        # killpg only exists on POSIX; on Windows use taskkill /T instead.
+        if hasattr(os, "killpg") and isinstance(pid, int) and pid > 0:
             try:
                 os.killpg(pid, signal.SIGKILL)
                 return
             except (ProcessLookupError, PermissionError, OSError):
                 pass
+        if os.name == "nt" and isinstance(pid, int) and pid > 0:
+            # proc.kill() terminates only the direct child. CLI shims
+            # (.cmd -> cmd.exe -> node/bun) do their real work in
+            # grandchildren that inherit the output pipes; if they outlive
+            # the parent they hold the pipes open and asyncio's proc.wait()
+            # — which resolves only after every pipe disconnects — blocks
+            # indefinitely. taskkill /T is the closest analog to killpg.
+            try:
+                subprocess.run(
+                    ["taskkill", "/F", "/T", "/PID", str(pid)],
+                    capture_output=True,
+                    timeout=5,
+                    check=False,
+                )
+            except (OSError, subprocess.SubprocessError):
+                pass
         try:
             proc.kill()
         except ProcessLookupError:
             pass
 
     timed_out = False
     idle_timed_out = False
+    completed = False
     deadline = asyncio.get_event_loop().time() + timeout if timeout else None
 
     try:
@@ -135,6 +201,7 @@ def _kill_group() -> None:
 
             try:
                 await asyncio.wait_for(asyncio.shield(drain), timeout=wait_for)
+                completed = True
                 break  # both pipes hit EOF: child is done
             except asyncio.TimeoutError:
                 now = asyncio.get_event_loop().time()
@@ -146,14 +213,18 @@ def _kill_group() -> None:
                     break
                 # Spurious wakeup (progress reset the idle window): loop again.
     finally:
-        if timed_out or idle_timed_out:
+        if not completed:
+            # Timeout, cancellation, or an internal error: stop the child so
+            # nothing keeps running and the exit wait below can conclude.
+            # (Previously only the timeout paths killed; a cancelled run_cli
+            # left the child running and awaited its natural exit.)
             _kill_group()
         drain.cancel()
         try:
             await drain
         except BaseException:
             pass
-        await proc.wait()
+        fallback_returncode = await _wait_process_exit(proc, _kill_group)
 
     if idle_timed_out:
         raise TimeoutError(
@@ -162,10 +233,13 @@ def _kill_group() -> None:
     if timed_out:
         raise TimeoutError(f"CLI command timed out after {timeout}s: {' '.join(cmd)}")
 
+    returncode = proc.returncode
+    if returncode is None:
+        returncode = fallback_returncode
     return (
         b"".join(stdout_chunks).decode("utf-8", errors="replace"),
         b"".join(stderr_chunks).decode("utf-8", errors="replace"),
-        proc.returncode if proc.returncode is not None else -1,
+        returncode if returncode is not None else -1,
     )
 
 

sdk/python/tests/test_harness_cli.py

@@ -58,6 +58,71 @@ async def test_run_cli_success():
     assert kwargs["stderr"] is asyncio.subprocess.PIPE
 
 
+@pytest.mark.asyncio
+async def test_run_cli_survives_lost_exit_notification(monkeypatch):
+    """Regression: CPython gh-81562 — Windows' proactor loop can lose the
+    RegisterWaitWithQueue completion, so ``proc.wait()`` never resolves even
+    though the child exited (both pipes at EOF). run_cli used to park there
+    forever, silently wedging the calling reasoner. It must instead recover
+    the exit status from the underlying Popen object and return."""
+    from agentfield.harness import _cli
+
+    monkeypatch.setattr(_cli, "_PROC_EXIT_GRACE_SECONDS", 0.05)
+
+    async def _never_resolves():
+        await asyncio.Event().wait()
+
+    process = MagicMock()
+    process.pid = 2147483647  # nonexistent pid: group kill falls back to kill()
+    process.stdout = _stream_reader([b"partial output"])
+    process.stderr = _stream_reader([])
+    process.returncode = None  # transport never learns the exit status
+    process.wait = _never_resolves
+    process.kill = MagicMock()
+    # Underlying Popen: first poll still racing, then the real exit code.
+    process._transport._proc.poll = MagicMock(side_effect=[None, 7])
+
+    with patch("asyncio.create_subprocess_exec", AsyncMock(return_value=process)):
+        stdout, stderr, returncode = await asyncio.wait_for(
+            run_cli(["opencode", "run"], timeout=5),
+            timeout=5,  # the whole call must conclude promptly, not hang
+        )
+
+    assert stdout == "partial output"
+    assert returncode == 7
+    process.kill.assert_called()
+
+
+@pytest.mark.asyncio
+async def test_run_cli_kills_child_on_cancellation():
+    """A cancelled run_cli must kill the child instead of leaving it running
+    and awaiting its natural exit (which for a coding-agent CLI can be many
+    minutes away, or forever)."""
+
+    async def never_ready(_n):
+        await asyncio.sleep(30)
+        return b""
+
+    process = MagicMock()
+    process.pid = 2147483647
+    process.returncode = None
+    process.stdout = MagicMock(read=AsyncMock(side_effect=never_ready))
+    process.stderr = MagicMock(read=AsyncMock(side_effect=never_ready))
+    process.kill = MagicMock()
+    process.wait = AsyncMock(return_value=1)
+
+    with patch("asyncio.create_subprocess_exec", AsyncMock(return_value=process)):
+        task = asyncio.ensure_future(
+            run_cli(["opencode", "run"], timeout=60, idle_seconds=0)
+        )
+        await asyncio.sleep(0.05)  # let it spawn and park on the drain
+        task.cancel()
+        with pytest.raises(asyncio.CancelledError):
+            await task
+
+    process.kill.assert_called()
+
+
 @pytest.mark.asyncio
 async def test_run_cli_timeout():
     async def never_ready(_n):

sdk/python/tests/test_run_cli_env.py

@@ -14,6 +14,7 @@
 from __future__ import annotations
 
 import asyncio
+import sys
 import time
 
 import pytest
@@ -175,3 +176,33 @@ async def test_run_cli_idle_seconds_from_env(monkeypatch):
         )
     elapsed = time.monotonic() - start
     assert elapsed < 10.0, f"env idle watchdog did not fire promptly ({elapsed:.1f}s)"
+
+
+@pytest.mark.asyncio
+async def test_run_cli_bounded_when_grandchild_holds_pipes():
+    """Functional regression for the silent-wedge class (real subprocesses).
+
+    A grandchild that inherits the output pipes and outlives its parent used
+    to park run_cli forever at the exit wait: asyncio resolves ``proc.wait()``
+    only once every pipe disconnects, and on Windows the group kill cannot
+    reach an already-orphaned grandchild. Observed in production as a
+    swe-planner reasoner stuck ``running`` for 26 minutes with no harness
+    process and a healthy event loop. run_cli must now conclude in bounded
+    time on every platform.
+    """
+    parent = (
+        "import subprocess, sys; "
+        "subprocess.Popen([sys.executable, '-c', 'import time; time.sleep(120)']); "
+        "print('parent-exiting', flush=True)"
+    )
+    start = time.monotonic()
+    with pytest.raises(TimeoutError):
+        await run_cli(
+            [sys.executable, "-c", parent],
+            timeout=60.0,
+            idle_seconds=2.0,
+        )
+    elapsed = time.monotonic() - start
+    # POSIX: killpg reaps the whole group at the idle deadline (~2s).
+    # Windows: worst case idle (2s) + exit-notification grace (10s) + poll.
+    assert elapsed < 45.0, f"exit wait not bounded ({elapsed:.1f}s)"