Skip to content
Merged
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
53 changes: 20 additions & 33 deletions src/benchflow/acp/runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@
from benchflow.acp.client import ACPClient
from benchflow.acp.container_transport import ContainerTransport
from benchflow.acp.selection import selected_acp_transport
from benchflow.acp.timeout_cleanup import (
cancel_and_drain_prompt_task,
cancel_prompt_after_timeout,
)
from benchflow.acp.types import McpServerSpec
from benchflow.acp.watchdog import IdleWatchdog
from benchflow.agents.protocol import ACPSessionAdapter
Expand Down Expand Up @@ -65,7 +69,6 @@

_ACP_CONNECT_MAX_RETRIES = 3
_ACP_CONNECT_BASE_DELAY = 2.0
_PROMPT_CANCEL_DRAIN_TIMEOUT_SEC = 0.25
_ACP_HANDSHAKE_TIMEOUT_ENV = "BENCHFLOW_ACP_HANDSHAKE_TIMEOUT"
_ACP_HANDSHAKE_TIMEOUT_DEFAULT_SEC = 60.0
_OPENHANDS_DISABLE_SUBAGENTS_ENV = "BENCHFLOW_OPENHANDS_DISABLE_SUBAGENTS"
Expand Down Expand Up @@ -744,31 +747,6 @@ async def execute_prompts(
return trajectory, len(session.tool_calls)


async def _cancel_and_drain_prompt_task(prompt_task: asyncio.Task) -> bool:
if prompt_task.done():
return True
prompt_task.cancel()
done, _pending = await asyncio.wait(
{prompt_task}, timeout=_PROMPT_CANCEL_DRAIN_TIMEOUT_SEC
)
if done:
with contextlib.suppress(BaseException):
prompt_task.result()
return True

logger.warning(
"ACP prompt task did not finish within %.2fs after cancellation",
_PROMPT_CANCEL_DRAIN_TIMEOUT_SEC,
)

def _consume_prompt_result(task: asyncio.Task) -> None:
with contextlib.suppress(BaseException):
task.result()

prompt_task.add_done_callback(_consume_prompt_result)
return False


def _agent_prompt_timeout_error(session, timeout: int) -> AgentPromptTimeoutError:
session.mark_prompt_end()
pending_tool_call_ids = session.pending_tool_call_ids()
Expand Down Expand Up @@ -800,16 +778,19 @@ async def _prompt_with_wall_clock_budget(
):
"""Run a prompt until either it finishes or BenchFlow's budget expires."""
prompt_task = asyncio.create_task(acp_client.prompt(prompt))
cleanup_attempted = False
try:
done, _pending = await asyncio.wait({prompt_task}, timeout=timeout)
if done:
return prompt_task.result()
if await _cancel_and_drain_prompt_task(prompt_task):
cleanup_attempted = True
if await cancel_prompt_after_timeout(acp_client, prompt_task):
raise _agent_prompt_timeout_error(session, timeout)
raise TimeoutError(f"Agent prompt exceeded wall-clock budget {timeout}s")
finally:
if not prompt_task.done():
await _cancel_and_drain_prompt_task(prompt_task)
if not prompt_task.done() and not cleanup_attempted:
cleanup_attempted = True
await cancel_and_drain_prompt_task(prompt_task)


async def _prompt_with_idle_watchdog(
Expand All @@ -821,6 +802,7 @@ async def _prompt_with_idle_watchdog(
):
"""Run one ACP prompt with wall-clock and idle-watchdog budgets."""
prompt_task = asyncio.create_task(acp_client.prompt(prompt))
cleanup_attempted = False
loop = asyncio.get_running_loop()
watchdog = IdleWatchdog.start(
session,
Expand All @@ -840,9 +822,13 @@ async def _prompt_with_idle_watchdog(
now = loop.time()
watchdog.observe(session, now=now)
if watchdog.idle_expired(now):
raise watchdog.timeout_error(session, now=now)
error = watchdog.timeout_error(session, now=now)
cleanup_attempted = True
await cancel_prompt_after_timeout(acp_client, prompt_task)
raise error
if watchdog.wall_clock_expired(now):
if await _cancel_and_drain_prompt_task(prompt_task):
cleanup_attempted = True
if await cancel_prompt_after_timeout(acp_client, prompt_task):
raise _agent_prompt_timeout_error(session, timeout)
raise TimeoutError(
f"Agent prompt exceeded wall-clock budget {timeout}s"
Expand All @@ -854,5 +840,6 @@ async def _prompt_with_idle_watchdog(
# external-cancellation path (CancelledError from sleep). Bound the
# drain so a non-cooperative Daytona/ACP read cannot hide the watchdog
# timeout forever; cleanup will tear down the live process.
if not prompt_task.done():
await _cancel_and_drain_prompt_task(prompt_task)
if not prompt_task.done() and not cleanup_attempted:
cleanup_attempted = True
await cancel_and_drain_prompt_task(prompt_task)
90 changes: 90 additions & 0 deletions src/benchflow/acp/timeout_cleanup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
"""Bounded cooperative cleanup for timed-out ACP prompts.

The original prompt task remains the sole ACP response reader. A best-effort
``session/cancel`` request runs independently while the prompt gets a short
grace period to receive peer-authored terminal usage and tool updates. At the
shared deadline, every surviving task is cancelled and boundedly drained.
"""

from __future__ import annotations

import asyncio
import contextlib
import logging
from typing import Any

logger = logging.getLogger(__name__)

PROMPT_TIMEOUT_CLEANUP_TOTAL_SEC = 5.0
PROMPT_CANCEL_DRAIN_TIMEOUT_SEC = 0.25


def _consume_task_result(task: asyncio.Task) -> None:
with contextlib.suppress(BaseException):
task.result()


async def _cancel_and_drain_tasks(tasks: set[asyncio.Task], timeout: float) -> None:
pending = {task for task in tasks if not task.done()}
for task in pending:
task.cancel()
_done, pending = (
await asyncio.wait(pending, timeout=timeout) if pending else (set(), set())
)
for task in tasks - pending:
_consume_task_result(task)
for task in pending:
task.add_done_callback(_consume_task_result)


async def cancel_and_drain_prompt_task(prompt_task: asyncio.Task) -> bool:
"""Hard-cancel a prompt and return whether it completed within the drain."""
if prompt_task.done():
_consume_task_result(prompt_task)
return True
await _cancel_and_drain_tasks({prompt_task}, PROMPT_CANCEL_DRAIN_TIMEOUT_SEC)
if prompt_task.done():
return True

logger.warning(
"ACP prompt task did not finish within %.2fs after cancellation",
PROMPT_CANCEL_DRAIN_TIMEOUT_SEC,
)
return False


async def cancel_prompt_after_timeout(
acp_client: Any, prompt_task: asyncio.Task
) -> bool:
"""Request peer cancellation, then bound cleanup without adding a reader."""
cancel = getattr(acp_client, "cancel", None)
if not callable(cancel):
return await cancel_and_drain_prompt_task(prompt_task)
if prompt_task.done():
_consume_task_result(prompt_task)
return True

async def _request_cancel() -> None:
try:
await cancel()
except asyncio.CancelledError:
raise
except Exception:
logger.warning("Failed to request ACP prompt cancellation", exc_info=True)

loop = asyncio.get_running_loop()
deadline = loop.time() + PROMPT_TIMEOUT_CLEANUP_TOTAL_SEC
cancel_task = asyncio.create_task(_request_cancel())
try:
cooperative_timeout = max(
0.0,
deadline - loop.time() - PROMPT_CANCEL_DRAIN_TIMEOUT_SEC,
)
await asyncio.wait({prompt_task}, timeout=cooperative_timeout)
finally:
remaining = max(0.0, deadline - loop.time())
await _cancel_and_drain_tasks(
{prompt_task, cancel_task},
min(PROMPT_CANCEL_DRAIN_TIMEOUT_SEC, remaining),
)
return prompt_task.done()
4 changes: 4 additions & 0 deletions src/benchflow/rollout/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1371,6 +1371,9 @@ async def disconnect(self) -> None:
self._capture_partial_session_factory_trajectory()
else:
self._capture_partial_acp_trajectory()
collect_native_usage = getattr(self, "_collect_native_acp_usage", None)
if callable(collect_native_usage):
collect_native_usage()
if self._acp_client:
try:
await self._acp_client.close()
Expand Down Expand Up @@ -2385,6 +2388,7 @@ async def connect_as(self, role: Role) -> None:
role.agent, getattr(self, "_task", None), agent_cfg
),
)
self._native_usage_checkpoint = None
self._reapply_ask_user_handler()
self._attach_trajectory_writer(rollout_dir)
self._active_role = role
Expand Down
68 changes: 0 additions & 68 deletions tests/test_acp.py
Original file line number Diff line number Diff line change
Expand Up @@ -830,74 +830,6 @@ async def test_prompt_with_interleaved_notifications_and_request(self) -> None:
await client.close()


class TestACPIdleWatchdog:
@pytest.mark.asyncio
async def test_idle_watchdog_returns_even_when_prompt_cancel_drain_stalls(
self,
) -> None:
"""Guards the 2026-05-22 Daytona/Gemini blocker fix against stuck cancel drain.

When the idle watchdog fires it cancels the prompt task and drains it.
A non-cooperative agent — here one that swallows the cancellation and
blocks on an event that never arrives — must not be able to wedge the
watchdog: it bounds the drain and raises ``IdleTimeoutError`` anyway.

Determinism: this asserts the *behaviour* (idle error raised; the stuck
prompt task abandoned while still pending) rather than a wall-clock upper
bound, so it cannot be squeezed into a spurious failure when the full
suite loads the event loop. The outer ``wait_for`` is only a hang guard;
its timeout is deliberately loose (far above the real ~1.25s runtime), so
load can never trip it, while a regression to an unbounded drain hangs
past it and fails.
"""
from benchflow.acp.runtime import IdleTimeoutError, execute_prompts

# release is never set while the watchdog runs, so the prompt task stays
# wedged in its cancellation handler — exactly the stuck-drain scenario.
class StubbornPromptClient:
def __init__(self) -> None:
self.release = asyncio.Event()
self.task: asyncio.Task | None = None

async def prompt(self, _prompt: str):
self.task = asyncio.current_task()
try:
await asyncio.Future()
except asyncio.CancelledError:
await self.release.wait()
raise

client = StubbornPromptClient()
session = ACPSession("idle-session")

# Loose hang guard: ~4x the real runtime (1s idle detect + 0.25s bounded
# drain). Not a tight assertion — it only catches a true hang/regression.
hang_guard_sec = 5.0

try:
with pytest.raises(IdleTimeoutError, match="Agent idle for 1s"):
await asyncio.wait_for(
execute_prompts(
client, # type: ignore[arg-type]
session,
["solve"],
timeout=30,
idle_timeout=1,
),
timeout=hang_guard_sec,
)
# The watchdog returned while the stuck prompt task is still wedged on
# release.wait(): proves it bounded the drain instead of waiting for a
# cancellation that never completes. Load-independent — no clock math.
assert client.task is not None
assert not client.task.done()
finally:
client.release.set()
if client.task is not None:
with pytest.raises(asyncio.CancelledError):
await client.task


class TestPendingToolCallGrace:
@pytest.mark.asyncio
async def test_lost_tool_call_completion_trips_idle_after_grace(self):
Expand Down
Loading
Loading