mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-28 18:19:28 +00:00
fix(agent): harden non-finite wait recovery
Only advertise finite watchdog deadlines that are still in the future, exercise the full MoA heartbeat path, and register the salvaged contributor attribution.
This commit is contained in:
parent
1a5d2a12d8
commit
14ea8de763
3 changed files with 205 additions and 15 deletions
|
|
@ -201,6 +201,7 @@ def _codex_wait_notice_recovery(
|
|||
call_start: float,
|
||||
idle_enabled: bool,
|
||||
idle_timeout: float,
|
||||
elapsed: float,
|
||||
) -> str:
|
||||
"""Describe the earliest enabled Codex watchdog on the call timeline."""
|
||||
deadlines: list[float] = []
|
||||
|
|
@ -211,7 +212,7 @@ def _codex_wait_notice_recovery(
|
|||
deadlines.append(ttfb_timeout)
|
||||
elif idle_enabled and math.isfinite(idle_timeout):
|
||||
deadlines.append(max(0.0, last_event_ts - call_start) + idle_timeout)
|
||||
if not deadlines:
|
||||
if not deadlines or min(deadlines) <= elapsed:
|
||||
return ""
|
||||
return f"; auto-reconnect at {int(min(deadlines))}s"
|
||||
|
||||
|
|
@ -636,20 +637,26 @@ def interruptible_api_call(agent, api_kwargs: dict):
|
|||
# usually a slow/overloaded provider, but the UI never said so).
|
||||
if _poll_count % 100 == 0: # 100 × 0.3s = 30s
|
||||
_elapsed = time.time() - _call_start
|
||||
_recovery = _codex_wait_notice_recovery(
|
||||
stale_timeout=_stale_timeout,
|
||||
ttfb_enabled=_ttfb_enabled,
|
||||
ttfb_timeout=_ttfb_timeout,
|
||||
last_event_ts=getattr(agent, "_codex_stream_last_event_ts", None),
|
||||
call_start=_call_start,
|
||||
idle_enabled=_codex_idle_enabled,
|
||||
idle_timeout=_codex_idle_timeout,
|
||||
)
|
||||
agent._emit_wait_notice(
|
||||
f"⏳ waiting on {api_kwargs.get('model', 'the provider')} — "
|
||||
f"{int(_elapsed)}s with no response yet (provider may be slow "
|
||||
f"or overloaded{_recovery})"
|
||||
)
|
||||
try:
|
||||
_recovery = _codex_wait_notice_recovery(
|
||||
stale_timeout=_stale_timeout,
|
||||
ttfb_enabled=_ttfb_enabled,
|
||||
ttfb_timeout=_ttfb_timeout,
|
||||
last_event_ts=getattr(
|
||||
agent, "_codex_stream_last_event_ts", None
|
||||
),
|
||||
call_start=_call_start,
|
||||
idle_enabled=_codex_idle_enabled,
|
||||
idle_timeout=_codex_idle_timeout,
|
||||
elapsed=_elapsed,
|
||||
)
|
||||
agent._emit_wait_notice(
|
||||
f"⏳ waiting on {api_kwargs.get('model', 'the provider')} — "
|
||||
f"{int(_elapsed)}s with no response yet (provider may be slow "
|
||||
f"or overloaded{_recovery})"
|
||||
)
|
||||
except Exception:
|
||||
logger.debug("wait-notice construction failed", exc_info=True)
|
||||
|
||||
_elapsed = time.time() - _call_start
|
||||
|
||||
|
|
|
|||
|
|
@ -45,6 +45,7 @@ ACP_REGISTRY_MANIFEST = REPO_ROOT / "acp_registry" / "agent.json"
|
|||
|
||||
# Auto-extracted from noreply emails + manual overrides
|
||||
AUTHOR_MAP = {
|
||||
"122438640+ragingbulld@users.noreply.github.com": "ragingbulld", # PR #65606 salvage (non-finite API wait deadlines; #65746)
|
||||
"zzpigpinggai@users.noreply.github.com": "zzpigpinggai", # PR #66017 salvage of #63617 (OpenRouter explicit-provider picker visibility)
|
||||
"sam7894604@gmail.com": "sam7894604", # PR #55803 salvage (discord: /reasoning slash choices)
|
||||
"bryan@users.noreply.github.com": "hydraxman", # PR #62028 salvage (copilot xhigh) — regression-test commit authored under a bare-noreply local git identity; PR author is @hydraxman
|
||||
|
|
|
|||
|
|
@ -329,11 +329,193 @@ def test_wait_notice_handles_infinite_local_stale_timeout():
|
|||
call_start=100.0,
|
||||
idle_enabled=True,
|
||||
idle_timeout=60.0,
|
||||
elapsed=30.0,
|
||||
)
|
||||
|
||||
assert recovery == "; auto-reconnect at 90s"
|
||||
|
||||
|
||||
def test_wait_notice_reports_ttfb_before_first_event():
|
||||
"""Before the first SSE event, the finite TTFB cutoff is the recovery."""
|
||||
from agent import chat_completion_helpers as h
|
||||
|
||||
recovery = h._codex_wait_notice_recovery(
|
||||
stale_timeout=float("inf"),
|
||||
ttfb_enabled=True,
|
||||
ttfb_timeout=120.0,
|
||||
last_event_ts=None,
|
||||
call_start=100.0,
|
||||
idle_enabled=True,
|
||||
idle_timeout=60.0,
|
||||
elapsed=30.0,
|
||||
)
|
||||
|
||||
assert recovery == "; auto-reconnect at 120s"
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"stale_timeout",
|
||||
[float("inf"), float("-inf"), float("nan")],
|
||||
)
|
||||
def test_wait_notice_omits_reconnect_when_all_deadlines_are_non_finite(
|
||||
stale_timeout,
|
||||
):
|
||||
"""A disabled watchdog must not be advertised as a future reconnect."""
|
||||
from agent import chat_completion_helpers as h
|
||||
|
||||
recovery = h._codex_wait_notice_recovery(
|
||||
stale_timeout=stale_timeout,
|
||||
ttfb_enabled=False,
|
||||
ttfb_timeout=float("nan"),
|
||||
last_event_ts=None,
|
||||
call_start=100.0,
|
||||
idle_enabled=False,
|
||||
idle_timeout=float("nan"),
|
||||
elapsed=30.0,
|
||||
)
|
||||
|
||||
assert recovery == ""
|
||||
|
||||
|
||||
def test_wait_notice_omits_elapsed_idle_deadline():
|
||||
"""An idle watchdog that already expired must not claim future recovery."""
|
||||
from agent import chat_completion_helpers as h
|
||||
|
||||
recovery = h._codex_wait_notice_recovery(
|
||||
stale_timeout=float("inf"),
|
||||
ttfb_enabled=True,
|
||||
ttfb_timeout=120.0,
|
||||
last_event_ts=100.0,
|
||||
call_start=100.0,
|
||||
idle_enabled=True,
|
||||
idle_timeout=30.0,
|
||||
elapsed=60.0,
|
||||
)
|
||||
|
||||
assert recovery == ""
|
||||
|
||||
|
||||
def test_wait_notice_does_not_skip_elapsed_stale_deadline_for_later_idle():
|
||||
"""An already-due watchdog wins; do not advertise a later deadline."""
|
||||
from agent import chat_completion_helpers as h
|
||||
|
||||
recovery = h._codex_wait_notice_recovery(
|
||||
stale_timeout=30.0,
|
||||
ttfb_enabled=True,
|
||||
ttfb_timeout=120.0,
|
||||
last_event_ts=130.0,
|
||||
call_start=100.0,
|
||||
idle_enabled=True,
|
||||
idle_timeout=60.0,
|
||||
elapsed=60.0,
|
||||
)
|
||||
|
||||
assert recovery == ""
|
||||
|
||||
|
||||
def test_moa_heartbeat_survives_infinite_stale_timeout(monkeypatch):
|
||||
"""The full 100-poll MoA heartbeat must leave a healthy call running."""
|
||||
from agent import chat_completion_helpers as h
|
||||
|
||||
notices: list[str] = []
|
||||
response = SimpleNamespace(ok=True)
|
||||
agent = SimpleNamespace(
|
||||
platform="desktop",
|
||||
api_mode="chat_completions",
|
||||
provider="moa",
|
||||
_consecutive_stale_streams=0,
|
||||
_interrupt_requested=False,
|
||||
_compute_non_stream_stale_timeout=lambda _kwargs: float("inf"),
|
||||
_touch_activity=lambda _message: None,
|
||||
_emit_wait_notice=notices.append,
|
||||
)
|
||||
|
||||
class HeartbeatThread:
|
||||
"""Keep the synthetic worker alive through one heartbeat."""
|
||||
|
||||
def __init__(self, *, target, daemon):
|
||||
self._polls = 0
|
||||
self._target = target
|
||||
|
||||
def start(self):
|
||||
pass
|
||||
|
||||
def join(self, timeout=None):
|
||||
pass
|
||||
|
||||
def is_alive(self):
|
||||
self._polls += 1
|
||||
if self._polls == 101:
|
||||
self._target()
|
||||
return False
|
||||
return True
|
||||
|
||||
monkeypatch.setattr(h.threading, "Thread", HeartbeatThread)
|
||||
monkeypatch.setattr(
|
||||
h,
|
||||
"_dispatch_nonstreaming_api_request",
|
||||
lambda *_args, **_kwargs: response,
|
||||
)
|
||||
|
||||
result = h.interruptible_api_call(agent, {"model": "openai-xai-wide"})
|
||||
|
||||
assert result is response
|
||||
assert len(notices) == 1
|
||||
assert "waiting on openai-xai-wide" in notices[0]
|
||||
assert "auto-reconnect" not in notices[0]
|
||||
|
||||
|
||||
def test_wait_notice_formatting_error_does_not_abort_request(monkeypatch):
|
||||
"""Status construction is fail-open even if its formatter breaks."""
|
||||
from agent import chat_completion_helpers as h
|
||||
|
||||
response = SimpleNamespace(ok=True)
|
||||
agent = SimpleNamespace(
|
||||
platform="desktop",
|
||||
api_mode="chat_completions",
|
||||
provider="moa",
|
||||
_consecutive_stale_streams=0,
|
||||
_interrupt_requested=False,
|
||||
_compute_non_stream_stale_timeout=lambda _kwargs: float("inf"),
|
||||
_touch_activity=lambda _message: None,
|
||||
_emit_wait_notice=lambda _message: None,
|
||||
)
|
||||
|
||||
class HeartbeatThread:
|
||||
def __init__(self, *, target, daemon):
|
||||
self._polls = 0
|
||||
self._target = target
|
||||
|
||||
def start(self):
|
||||
pass
|
||||
|
||||
def join(self, timeout=None):
|
||||
pass
|
||||
|
||||
def is_alive(self):
|
||||
self._polls += 1
|
||||
if self._polls == 101:
|
||||
self._target()
|
||||
return False
|
||||
return True
|
||||
|
||||
monkeypatch.setattr(h.threading, "Thread", HeartbeatThread)
|
||||
monkeypatch.setattr(
|
||||
h,
|
||||
"_dispatch_nonstreaming_api_request",
|
||||
lambda *_args, **_kwargs: response,
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
h,
|
||||
"_codex_wait_notice_recovery",
|
||||
lambda **_kwargs: (_ for _ in ()).throw(ValueError("bad display state")),
|
||||
)
|
||||
|
||||
result = h.interruptible_api_call(agent, {"model": "openai-xai-wide"})
|
||||
|
||||
assert result is response
|
||||
|
||||
|
||||
def test_ttfb_disabled_via_env_zero(tmp_path, monkeypatch):
|
||||
"""Setting HERMES_CODEX_TTFB_TIMEOUT_SECONDS=0 disables the TTFB watchdog;
|
||||
a no-event stall then falls through to the (here, 60s) stale timeout, so a
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue