mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-15 14:22:43 +00:00
fix(gateway): harden fallback-chain refresh from review findings
Follow-ups on the #60987 salvage (review pass): - _refresh_fallback_model: keep last known-good chain on transient config.yaml read/parse failure (user mid-edit, torn write) — only a successful read that lacks the key clears the chain. Previously a refresh error wiped a cached agent's working fallback for the turn. - Move the cached-agent refresh+apply OUTSIDE the agent-cache lock: config.yaml read is disk I/O and the idle-sweep watcher contends on that lock (same reasoning as #52197). Per-session turn serialization keeps the post-lock apply safe. - _apply_fallback_chain_to_agent: clear _unavailable_fallback_keys when chain content actually changes, so an entry re-configured mid-uptime (e.g. credentials added) is retried instead of staying suppressed for the cached agent's lifetime; no-op refreshes keep the memo. - Tests: cwd-independent source pin (Path(__file__) anchor), pin the reuse-path apply call, + regression tests for last-known-good, memo clear-on-change, memo keep-on-unchanged (mutation-verified).
This commit is contained in:
parent
e721ad89e3
commit
0a01b2087d
2 changed files with 122 additions and 10 deletions
|
|
@ -5016,8 +5016,28 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
|||
configured (or changed) after ``hermes gateway`` was running never
|
||||
reached messaging sessions even though the same process's cron jobs
|
||||
fell back correctly. Fixes #60955.
|
||||
|
||||
A TRANSIENT read/parse failure (user mid-edit of config.yaml with a
|
||||
non-atomic write) keeps the last known-good chain instead of wiping a
|
||||
cached agent's working fallback for that turn. Only a successful read
|
||||
that genuinely lacks the key clears the chain.
|
||||
"""
|
||||
self._fallback_model = self._load_fallback_model()
|
||||
try:
|
||||
import yaml as _y
|
||||
cfg_path = _hermes_home / "config.yaml"
|
||||
if not cfg_path.exists():
|
||||
self._fallback_model = None
|
||||
return self._fallback_model
|
||||
with open(cfg_path, encoding="utf-8") as _f:
|
||||
cfg = _y.safe_load(_f) or {}
|
||||
except Exception:
|
||||
# Transient failure — keep last known-good chain.
|
||||
logger.debug(
|
||||
"fallback_providers refresh: config.yaml read failed; "
|
||||
"keeping last known-good chain", exc_info=True,
|
||||
)
|
||||
return self._fallback_model
|
||||
self._fallback_model = get_fallback_chain(cfg) or None
|
||||
return self._fallback_model
|
||||
|
||||
@staticmethod
|
||||
|
|
@ -5039,10 +5059,22 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
|||
and rate_limited_until > time.monotonic()
|
||||
):
|
||||
return
|
||||
old_chain = list(getattr(agent, "_fallback_chain", []) or [])
|
||||
agent._fallback_chain = new_chain
|
||||
agent._fallback_model = new_chain[0] if new_chain else None
|
||||
if not getattr(agent, "_fallback_activated", False):
|
||||
agent._fallback_index = 0
|
||||
# A config edit signals the user changed something — drop the
|
||||
# session-scoped unavailability memo so re-configured entries
|
||||
# (e.g. credentials added mid-uptime for a previously-failing
|
||||
# provider) get retried instead of staying suppressed for the
|
||||
# cached agent's lifetime. Only on actual content change, so
|
||||
# the per-message no-op refresh keeps the memo's rate-limiting
|
||||
# benefit (#60955).
|
||||
if new_chain != old_chain:
|
||||
unavailable = getattr(agent, "_unavailable_fallback_keys", None)
|
||||
if unavailable:
|
||||
unavailable.clear()
|
||||
|
||||
def _snapshot_running_agents(self) -> Dict[str, Any]:
|
||||
return {
|
||||
|
|
@ -18099,15 +18131,21 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
|||
# Refresh agent max_iterations from current config
|
||||
# (cached agent may have been created with old config)
|
||||
agent.max_iterations = max_iterations
|
||||
# Refresh fallback chain the same way — a chain
|
||||
# configured after this agent was cached (or after
|
||||
# gateway start) must reach the next turn (#60955).
|
||||
self._apply_fallback_chain_to_agent(
|
||||
agent, self._refresh_fallback_model(),
|
||||
)
|
||||
logger.debug("Reusing cached agent for session %s", session_key)
|
||||
reused_cached_agent = True
|
||||
|
||||
# Lock released — refresh the fallback chain from disk for the
|
||||
# reused agent OUTSIDE the cache lock (config.yaml read is disk
|
||||
# I/O; the idle-sweep watcher contends on this lock and stalls
|
||||
# Discord heartbeats — same reasoning as #52197). A chain
|
||||
# configured after this agent was cached (or after gateway start)
|
||||
# must reach the next turn (#60955). Per-session turn
|
||||
# serialization (_running_agents) keeps this safe post-lock.
|
||||
if reused_cached_agent and agent is not None:
|
||||
self._apply_fallback_chain_to_agent(
|
||||
agent, self._refresh_fallback_model(),
|
||||
)
|
||||
|
||||
# Lock released — now schedule cleanup of any cross-process-evicted
|
||||
# agent on a daemon thread so memory-provider shutdown / socket
|
||||
# teardown never blocks the gateway event loop or the cache lock
|
||||
|
|
|
|||
|
|
@ -71,6 +71,34 @@ def test_refresh_fallback_model_clears_when_config_removed(tmp_path, monkeypatch
|
|||
assert runner._fallback_model is None
|
||||
|
||||
|
||||
def test_refresh_fallback_model_keeps_last_known_good_on_read_failure(
|
||||
tmp_path, monkeypatch,
|
||||
):
|
||||
"""A transient config.yaml read/parse failure (user mid-edit, non-atomic
|
||||
write) must NOT wipe the last known-good chain — only a successful read
|
||||
that genuinely lacks the key clears it."""
|
||||
from gateway.run import GatewayRunner
|
||||
|
||||
monkeypatch.setattr("gateway.run._hermes_home", tmp_path)
|
||||
cfg = tmp_path / "config.yaml"
|
||||
cfg.write_text(
|
||||
"fallback_providers:\n"
|
||||
" - provider: deepseek\n"
|
||||
" model: deepseek-v4-flash\n"
|
||||
)
|
||||
|
||||
runner = SimpleNamespace(_fallback_model=None)
|
||||
runner._load_fallback_model = GatewayRunner._load_fallback_model
|
||||
bound = GatewayRunner._refresh_fallback_model.__get__(runner)
|
||||
good = bound()
|
||||
assert good == [{"provider": "deepseek", "model": "deepseek-v4-flash"}]
|
||||
|
||||
# Simulate a mid-edit torn write: invalid YAML.
|
||||
cfg.write_text("fallback_providers:\n - provider: [unclosed\n")
|
||||
assert bound() == good
|
||||
assert runner._fallback_model == good
|
||||
|
||||
|
||||
def test_apply_fallback_chain_updates_primary_agent():
|
||||
from gateway.run import GatewayRunner
|
||||
|
||||
|
|
@ -130,16 +158,62 @@ def test_apply_fallback_chain_updates_after_cooldown_expires():
|
|||
assert agent._fallback_index == 1
|
||||
|
||||
|
||||
def test_apply_fallback_chain_clears_unavailable_memo_on_content_change():
|
||||
"""A config edit must drop the session-scoped unavailability memo so a
|
||||
re-configured entry (credentials added mid-uptime) is retried instead of
|
||||
staying suppressed for the cached agent's lifetime."""
|
||||
from gateway.run import GatewayRunner
|
||||
|
||||
agent = SimpleNamespace(
|
||||
_fallback_chain=[{"provider": "deepseek", "model": "old"}],
|
||||
_fallback_model={"provider": "deepseek", "model": "old"},
|
||||
_fallback_index=0,
|
||||
_fallback_activated=False,
|
||||
_rate_limited_until=0,
|
||||
_unavailable_fallback_keys={("deepseek", "old", "")},
|
||||
)
|
||||
new_chain = [{"provider": "deepseek", "model": "deepseek-v4-flash"}]
|
||||
GatewayRunner._apply_fallback_chain_to_agent(agent, new_chain)
|
||||
|
||||
assert agent._fallback_chain == new_chain
|
||||
assert agent._unavailable_fallback_keys == set()
|
||||
|
||||
|
||||
def test_apply_fallback_chain_keeps_unavailable_memo_when_unchanged():
|
||||
"""The per-message no-op refresh must NOT clear the memo — it exists to
|
||||
rate-limit repeated activation attempts against dead entries."""
|
||||
from gateway.run import GatewayRunner
|
||||
|
||||
chain = [{"provider": "deepseek", "model": "deepseek-v4-flash"}]
|
||||
memo = {("deepseek", "deepseek-v4-flash", "")}
|
||||
agent = SimpleNamespace(
|
||||
_fallback_chain=list(chain),
|
||||
_fallback_model=chain[0],
|
||||
_fallback_index=0,
|
||||
_fallback_activated=False,
|
||||
_rate_limited_until=0,
|
||||
_unavailable_fallback_keys=set(memo),
|
||||
)
|
||||
GatewayRunner._apply_fallback_chain_to_agent(agent, list(chain))
|
||||
|
||||
assert agent._unavailable_fallback_keys == memo
|
||||
|
||||
|
||||
def test_background_and_main_agent_paths_call_refresh():
|
||||
"""Both AIAgent construction sites must pass a refreshed chain, not the
|
||||
startup snapshot. Source-level invariant — mirrors
|
||||
tests/agent/test_gemini_fast_fallback.py's call-site pin.
|
||||
startup snapshot, and the cached-agent reuse path must apply the refreshed
|
||||
chain. Source-level invariant for call sites that resist unit testing.
|
||||
"""
|
||||
from pathlib import Path
|
||||
|
||||
source = Path("gateway/run.py").read_text(encoding="utf-8")
|
||||
source = (
|
||||
Path(__file__).resolve().parent.parent.parent / "gateway" / "run.py"
|
||||
).read_text(encoding="utf-8")
|
||||
assert "fallback_model=self._refresh_fallback_model()" in source
|
||||
assert source.count("fallback_model=self._refresh_fallback_model()") >= 2
|
||||
# The cached-agent reuse path (the load-bearing fix for a long-lived
|
||||
# session in a running gateway) must apply the refreshed chain.
|
||||
assert "self._apply_fallback_chain_to_agent(" in source
|
||||
# The stale startup-snapshot form must not remain at create sites.
|
||||
assert "fallback_model=self._fallback_model," not in source
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue