From 7bc7937c080ab08ffd7f98f0725c290d48c5de56 Mon Sep 17 00:00:00 2001 From: liuhao1024 Date: Sat, 19 Sep 2026 18:03:44 +0800 Subject: [PATCH] fix(agent): release the portal account fetch caller at the wall-clock bound --- agent/account_usage.py | 23 ++++++++-- tests/agent/test_account_usage_fetch.py | 59 +++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 4 deletions(-) diff --git a/agent/account_usage.py b/agent/account_usage.py index 579346a142..fde2c29e3c 100644 --- a/agent/account_usage.py +++ b/agent/account_usage.py @@ -187,13 +187,28 @@ def _nous_logged_in() -> bool: def _fetch_portal_account(timeout: float): - """Wall-clock-bounded fresh portal account fetch (raises on any failure/timeout).""" - import concurrent.futures + """Wall-clock-bounded fresh portal account fetch (raises on any failure/timeout). + + No ``with`` block on purpose: ``Executor.__exit__`` joins the worker via + ``shutdown(wait=True)``, so a portal that accepts the connection but never + answers would hold the caller until the provider's own timeout instead of + ``timeout``. The abandoned daemon worker runs on to its own network timeout + and never blocks the caller or process exit; its eventual exception is + drained so GC never logs "exception was never retrieved".""" import contextvars from hermes_cli.nous_account import get_nous_portal_account_info + from tools.daemon_pool import DaemonThreadPoolExecutor + context = contextvars.copy_context() - with concurrent.futures.ThreadPoolExecutor(max_workers=1) as pool: - return pool.submit(context.run, get_nous_portal_account_info, force_fresh=True).result(timeout=timeout) + pool = DaemonThreadPoolExecutor(max_workers=1) + future = pool.submit(context.run, get_nous_portal_account_info, force_fresh=True) + try: + return future.result(timeout=timeout) + except BaseException: + future.add_done_callback(lambda f: f.exception()) + raise + finally: + pool.shutdown(wait=False) def nous_credits_lines(*, markdown: bool = False, timeout: float = 10.0) -> list[str]: diff --git a/tests/agent/test_account_usage_fetch.py b/tests/agent/test_account_usage_fetch.py index e3086d1c4b..833d8fb53a 100644 --- a/tests/agent/test_account_usage_fetch.py +++ b/tests/agent/test_account_usage_fetch.py @@ -1,3 +1,7 @@ +import concurrent.futures +import contextvars +import threading +import time from datetime import datetime, timezone import pytest @@ -5,6 +9,7 @@ import pytest from agent.account_usage import ( AccountUsageSnapshot, AccountUsageWindow, + _fetch_portal_account, fetch_account_usage, render_account_usage_lines, ) @@ -325,3 +330,57 @@ def test_base_noop_usage_hook_spawns_no_thread(monkeypatch): lambda self: pytest.fail(f"base no-op hook spawned thread {self.name!r}")) assert account_usage.fetch_account_usage("plugin-noop") is None + + +def test_fetch_portal_account_is_wall_clock_bounded(monkeypatch): + """A portal that accepts the connection but never answers must release the + caller at ``timeout``, not when the wedged worker finishes on its own + (``Executor.__exit__`` used to join it via ``shutdown(wait=True)``).""" + release = threading.Event() + + def hanging_portal_fetch(*, force_fresh): + release.wait(timeout=30) + return object() + + monkeypatch.setattr( + "hermes_cli.nous_account.get_nous_portal_account_info", hanging_portal_fetch + ) + started = time.monotonic() + try: + with pytest.raises(concurrent.futures.TimeoutError): + _fetch_portal_account(timeout=0.5) + finally: + release.set() + assert time.monotonic() - started < 10 + + +def test_fetch_portal_account_returns_value_and_keeps_caller_context(monkeypatch): + marker = contextvars.ContextVar("portal_fetch_test_marker", default="unset") + sentinel = object() + seen = {} + + def probing_portal_fetch(*, force_fresh): + seen["force_fresh"] = force_fresh + seen["marker"] = marker.get() + return sentinel + + monkeypatch.setattr( + "hermes_cli.nous_account.get_nous_portal_account_info", probing_portal_fetch + ) + token = marker.set("profile-scope") + try: + assert _fetch_portal_account(timeout=5) is sentinel + finally: + marker.reset(token) + assert seen == {"force_fresh": True, "marker": "profile-scope"} + + +def test_fetch_portal_account_propagates_worker_error(monkeypatch): + def failing_portal_fetch(*, force_fresh): + raise RuntimeError("portal down") + + monkeypatch.setattr( + "hermes_cli.nous_account.get_nous_portal_account_info", failing_portal_fetch + ) + with pytest.raises(RuntimeError, match="portal down"): + _fetch_portal_account(timeout=5)