mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
fix(computer-use): report the wedged startup phase in the session ready-timeout error (#58801)
The 'never reached ready' error (issue #57025) was undiagnosable — doctor and MCP test pass while the wrapper times out, with no hint where startup stalled. Track a phase marker through _lifecycle_coro (binary-check → manifest-discovery → mcp-initialize → capability-discovery → ready) and include it in the timeout RuntimeError plus a pointer to doctor and the agent.log phase timings. Complements the 15s→30s bump + success-path phase timing log from #58760.
This commit is contained in:
parent
24a7546918
commit
de4310c8f6
2 changed files with 82 additions and 1 deletions
|
|
@ -2867,3 +2867,66 @@ class TestCuaToolCoverageExpansion:
|
|||
backend.call_tool("get_cursor_position")
|
||||
name, args = backend._session.call_tool.call_args.args
|
||||
assert args == {"session": backend._session_id}
|
||||
|
||||
|
||||
class TestStartupTimeoutPhaseDetail:
|
||||
"""Issue #57025: the ready-timeout error must report which startup phase
|
||||
wedged, so 'doctor passes but wrapper times out' reports are diagnosable."""
|
||||
|
||||
def test_timeout_error_includes_startup_phase(self):
|
||||
import threading
|
||||
from typing import Any, cast
|
||||
from unittest.mock import MagicMock
|
||||
from tools.computer_use.cua_backend import _CuaDriverSession
|
||||
|
||||
session = cast(Any, _CuaDriverSession.__new__(_CuaDriverSession))
|
||||
session._lock = threading.Lock()
|
||||
session._ready_event = threading.Event() # never set → timeout path
|
||||
session._setup_error = None
|
||||
session._shutdown_event = None
|
||||
session._startup_phase = "mcp-initialize"
|
||||
session._signal_shutdown_locked = lambda: None
|
||||
|
||||
fake_bridge = MagicMock()
|
||||
fake_bridge._loop = MagicMock()
|
||||
session._bridge = fake_bridge
|
||||
|
||||
import asyncio
|
||||
from unittest.mock import patch as _patch
|
||||
with _patch.object(session._ready_event, "wait", return_value=False), \
|
||||
_patch.object(asyncio, "run_coroutine_threadsafe", return_value=MagicMock()), \
|
||||
_patch.object(_CuaDriverSession, "_lifecycle_coro", lambda self: None):
|
||||
try:
|
||||
session._start_lifecycle_locked()
|
||||
assert False, "expected RuntimeError"
|
||||
except RuntimeError as e:
|
||||
msg = str(e)
|
||||
assert "stuck in phase: mcp-initialize" in msg
|
||||
assert "computer-use doctor" in msg
|
||||
|
||||
def test_timeout_error_defaults_to_unknown_phase(self):
|
||||
import threading
|
||||
from typing import Any, cast
|
||||
from unittest.mock import MagicMock, patch as _patch
|
||||
import asyncio
|
||||
from tools.computer_use.cua_backend import _CuaDriverSession
|
||||
|
||||
session = cast(Any, _CuaDriverSession.__new__(_CuaDriverSession))
|
||||
session._lock = threading.Lock()
|
||||
session._ready_event = threading.Event()
|
||||
session._setup_error = None
|
||||
session._shutdown_event = None
|
||||
# no _startup_phase attribute at all
|
||||
session._signal_shutdown_locked = lambda: None
|
||||
fake_bridge = MagicMock()
|
||||
fake_bridge._loop = MagicMock()
|
||||
session._bridge = fake_bridge
|
||||
|
||||
with _patch.object(session._ready_event, "wait", return_value=False), \
|
||||
_patch.object(asyncio, "run_coroutine_threadsafe", return_value=MagicMock()), \
|
||||
_patch.object(_CuaDriverSession, "_lifecycle_coro", lambda self: None):
|
||||
try:
|
||||
session._start_lifecycle_locked()
|
||||
assert False, "expected RuntimeError"
|
||||
except RuntimeError as e:
|
||||
assert "stuck in phase: unknown" in str(e)
|
||||
|
|
|
|||
|
|
@ -576,6 +576,10 @@ class _CuaDriverSession:
|
|||
# primitive belongs to the correct loop.
|
||||
self._shutdown_event = asyncio.Event()
|
||||
_t0 = _time.monotonic()
|
||||
# Phase marker surfaced by the ready-timeout error (issue #57025):
|
||||
# when startup wedges, the caller reports HOW FAR it got instead of
|
||||
# an opaque "never reached ready".
|
||||
self._startup_phase = "binary-check"
|
||||
|
||||
try:
|
||||
if not cua_driver_binary_available():
|
||||
|
|
@ -584,6 +588,7 @@ class _CuaDriverSession:
|
|||
# Surface 8: ask cua-driver itself which subcommand spawns
|
||||
# the MCP server, instead of hardcoding ["mcp"]. Falls back
|
||||
# transparently for older drivers / any discovery failure.
|
||||
self._startup_phase = "manifest-discovery"
|
||||
command, args = _resolve_mcp_invocation(_CUA_DRIVER_CMD)
|
||||
_t_manifest = _time.monotonic()
|
||||
params = StdioServerParameters(
|
||||
|
|
@ -595,14 +600,17 @@ class _CuaDriverSession:
|
|||
)
|
||||
|
||||
async with stdio_client(params) as (read, write):
|
||||
self._startup_phase = "mcp-initialize"
|
||||
async with ClientSession(read, write) as session:
|
||||
await session.initialize()
|
||||
_t_init = _time.monotonic()
|
||||
# Populate capabilities + capability_version BEFORE
|
||||
# exposing the session to callers, so the first
|
||||
# tool call already sees them.
|
||||
self._startup_phase = "capability-discovery"
|
||||
await self._populate_capabilities(session)
|
||||
self._session = session
|
||||
self._startup_phase = "ready"
|
||||
self._ready_event.set()
|
||||
logger.info(
|
||||
"cua-driver session ready in %.1fs "
|
||||
|
|
@ -691,7 +699,17 @@ class _CuaDriverSession:
|
|||
if not self._ready_event.wait(timeout=30.0):
|
||||
# Best-effort: signal shutdown if the future is still alive.
|
||||
self._signal_shutdown_locked()
|
||||
raise RuntimeError("cua-driver session never reached ready (timeout 30s)")
|
||||
# Surface which startup phase wedged (issue #57025) — "doctor
|
||||
# passes but the wrapper times out" reports are undiagnosable
|
||||
# from a bare "never reached ready".
|
||||
phase = getattr(self, "_startup_phase", "unknown")
|
||||
from hermes_constants import display_hermes_home
|
||||
raise RuntimeError(
|
||||
"cua-driver session never reached ready (timeout 30s; "
|
||||
f"stuck in phase: {phase}). "
|
||||
"Run `hermes computer-use doctor` and check "
|
||||
f"{display_hermes_home()}/logs/agent.log for the phase timings."
|
||||
)
|
||||
# If setup failed, the lifecycle coroutine set _setup_error
|
||||
# before setting _ready_event. Re-raise it on the caller's thread.
|
||||
if self._setup_error is not None:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue