Skip to content
26 changes: 20 additions & 6 deletions src/benchflow/acp/runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -841,6 +841,13 @@ def _activity_count() -> int:
last_progress = asyncio.get_event_loop().time()
last_activity_at = datetime.now(UTC)
last_count = _activity_count()
# A pending tool call defers the idle watchdog only within this grace: a
# completion update lost in transit (e.g. a half-open PTY websocket frame
# drop) leaves the call pending forever and would otherwise disarm the
# watchdog for the rest of the wall-clock budget (#1061).
pending_grace = idle_timeout * 3
pending_snapshot: tuple[str, ...] = ()
pending_since = last_progress
# poll_interval considers BOTH idle_timeout and wall-clock timeout so that
# short overall budgets don't overshoot (e.g. timeout=30s with default
# poll_interval=30s could overshoot 100%). Cap at 30s, floor at 1s.
Expand Down Expand Up @@ -870,13 +877,20 @@ def _activity_count() -> int:
# (e.g. a long build/test/solver shell command), not hung. Those tools
# emit no ACP updates until they return, so a >idle_timeout run would
# otherwise false-fire the watchdog and discard real work. Treat a
# pending tool call as progress and defer to the wall-clock `timeout`
# backstop below for a tool that never returns. A genuine model-side
# hang has no pending tool call (the prior tool already completed via
# tool_call_update), so it still trips the idle path.
# pending tool call as progress, but only within pending_grace of the
# pending set last changing: a call whose completion update was lost
# in transit stays pending forever, and an unbounded deferral would
# disarm the watchdog for the rest of the wall-clock budget (#1061).
# A genuine model-side hang has no pending tool call (the prior tool
# already completed via tool_call_update), so it trips the idle path.
elif session.pending_tool_call_ids():
last_progress = now
last_activity_at = datetime.now(UTC)
snapshot = tuple(sorted(session.pending_tool_call_ids()))
if snapshot != pending_snapshot:
pending_snapshot = snapshot
pending_since = now

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Delayed pending-call grace window

When a tool call appears between polls, cur_count > last_count postpones snapshot tracking until the next poll. The watchdog grants up to two extra poll intervals.

Prompt for agents
In src/benchflow/acp/runtime.py, _prompt_with_idle_watchdog only observes the pending-ID snapshot in the elif branch after activity-count processing. A newly appended tool call increases _activity_count, so the first poll that observes it skips snapshot tracking; pending_since is reset on the following poll instead. Track pending-set transitions on every poll, independently of whether other activity was also detected, while preserving the rule that a pending call only refreshes last_progress during its grace window. Add a regression test where the prompt coroutine creates the pending call after the watchdog starts, rather than pre-populating the session.
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, fixed: the pending set is now observed every poll, so the grace clock starts on the same poll the call appears.

if now - pending_since < pending_grace:
last_progress = now
last_activity_at = datetime.now(UTC)
if now - last_progress >= idle_timeout:
diag = IdleTimeoutDiagnostic(
idle_timeout_sec=idle_timeout,
Expand Down
35 changes: 35 additions & 0 deletions tests/test_acp.py
Original file line number Diff line number Diff line change
Expand Up @@ -898,6 +898,41 @@ async def prompt(self, _prompt: str):
await client.task


class TestPendingToolCallGrace:
@pytest.mark.asyncio
async def test_lost_tool_call_completion_trips_idle_after_grace(self):
"""Guards the #1061 fix: a tool call whose completion update never
arrives must stop deferring the idle watchdog once the pending grace
(3x idle_timeout) elapses with no other session activity, instead of
disarming it for the rest of the wall-clock budget.

Runtime: ~5s (3s grace + 1s idle + polls); the outer wait_for is a
loose hang guard, not a timing assertion.
"""
from benchflow.acp.runtime import IdleTimeoutError, execute_prompts
from benchflow.acp.session import ToolCallRecord

class HangingPromptClient:
async def prompt(self, _prompt: str):
await asyncio.Future()

session = ACPSession("pending-grace-session")
# A tool call stuck in PENDING with no terminal update, ever.
session.tool_calls.append(ToolCallRecord("t1", "sleep 180", "execute"))

with pytest.raises(IdleTimeoutError, match="Agent idle for 1s"):
await asyncio.wait_for(
execute_prompts(
HangingPromptClient(), # type: ignore[arg-type]
session,
["solve"],
timeout=60,
idle_timeout=1,
),
timeout=20.0,
)


class TestIdleTimeoutDiagnostics:
"""Guards ENG-149: idle timeouts must carry structured diagnostics."""

Expand Down