mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
fix(gateway): clear last-resolved-model cache on 3 more conversation-boundary resets
11b4a21a5cleared the per-session _last_resolved_model cache on /new and the compression-exhausted auto-reset, so a resumed/reset conversation resolves the model from current config instead of a stale cached value (#58403). Three other sites documented as the same "full conversation boundary" treatment — pop _session_model_overrides, clear the reasoning override, pop _pending_model_notes — still missed _last_resolved_model: - _session_expiry_watcher's permanent finalization block (gateway/run.py): a session that goes idle and is finalized, then resumed, could serve a model cached before it went idle on a transient config-cache miss. - The daily/idle/suspended auto-reset cleanup (_was_auto_reset handling, gateway/run.py): same failure mode, different trigger. - /resume (gateway/slash_commands.py), whose own comment already says "conversation boundary just like /new" for the sibling dicts it clears. Fix: pop the session's _last_resolved_model entry in all three, mirroring the exact pattern11b4a21a5established.
This commit is contained in:
parent
3817ff180d
commit
cdcbc3a31d
5 changed files with 174 additions and 0 deletions
|
|
@ -7534,6 +7534,13 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
|||
self._set_session_reasoning_override(key, None)
|
||||
if hasattr(self, "_pending_model_notes"):
|
||||
self._pending_model_notes.pop(key, None)
|
||||
# Clear per-session model cache so a resumed turn
|
||||
# resolves from current config, not a stale fallback
|
||||
# cached before the session went idle (mirrors /new
|
||||
# and the compression-exhausted auto-reset, #58403).
|
||||
_lrm = getattr(self, "_last_resolved_model", None)
|
||||
if _lrm is not None:
|
||||
_lrm.pop(key, None)
|
||||
_pending_approvals = getattr(self, "_pending_approvals", None)
|
||||
if isinstance(_pending_approvals, dict):
|
||||
_pending_approvals.pop(key, None)
|
||||
|
|
@ -10538,6 +10545,13 @@ class GatewayRunner(GatewayAuthorizationMixin, GatewayKanbanWatchersMixin, Gatew
|
|||
self._set_session_reasoning_override(session_key, None)
|
||||
if hasattr(self, "_pending_model_notes"):
|
||||
self._pending_model_notes.pop(session_key, None)
|
||||
# Clear per-session model cache so the fresh session resolves
|
||||
# from current config, not a stale fallback cached before the
|
||||
# auto-reset (mirrors /new and the compression-exhausted
|
||||
# auto-reset, #58403).
|
||||
_lrm = getattr(self, "_last_resolved_model", None)
|
||||
if _lrm is not None:
|
||||
_lrm.pop(session_key, None)
|
||||
# Evict the cached agent so the fresh session does not inherit the
|
||||
# previous conversation's context_compressor._previous_summary —
|
||||
# the cache is keyed on the stable session_key, so an auto-reset
|
||||
|
|
|
|||
|
|
@ -3632,6 +3632,13 @@ class GatewaySlashCommandsMixin:
|
|||
_pending_notes = getattr(self, "_pending_model_notes", None)
|
||||
if isinstance(_pending_notes, dict):
|
||||
_pending_notes.pop(session_key, None)
|
||||
# Clear per-session model cache too, for the same reason — the
|
||||
# resumed conversation must resolve from current config, not a
|
||||
# stale value cached under this session_key before the switch
|
||||
# (mirrors /new and the compression-exhausted auto-reset, #58403).
|
||||
_lrm = getattr(self, "_last_resolved_model", None)
|
||||
if isinstance(_lrm, dict):
|
||||
_lrm.pop(session_key, None)
|
||||
|
||||
# Evict any cached agent for this session so the next message
|
||||
# rebuilds with the correct session_id end-to-end — mirrors
|
||||
|
|
|
|||
|
|
@ -90,3 +90,49 @@ def test_evict_cached_agent_method_exists():
|
|||
"GatewayRunner._evict_cached_agent is the helper the auto-reset "
|
||||
"cleanup depends on (#10710)."
|
||||
)
|
||||
|
||||
|
||||
def _references_name(node: ast.AST, literal: str) -> bool:
|
||||
"""True if a string constant equal to ``literal`` appears anywhere under ``node``."""
|
||||
return any(
|
||||
isinstance(n, ast.Constant) and n.value == literal for n in ast.walk(node)
|
||||
)
|
||||
|
||||
|
||||
def test_auto_reset_cleanup_clears_last_resolved_model():
|
||||
"""Regression test for #58403.
|
||||
|
||||
The auto-reset cleanup block (daily/idle/suspended, fingerprinted by
|
||||
`_set_session_reasoning_override` + `was_auto_reset = False`) already
|
||||
pops `_session_model_overrides` and `_pending_model_notes` — the same
|
||||
"full conversation boundary" treatment /new and the compression-exhausted
|
||||
auto-reset give `_last_resolved_model`. Without also popping
|
||||
`_last_resolved_model` here, the fresh auto-reset session could serve a
|
||||
model cached before the reset on a transient config-cache miss.
|
||||
"""
|
||||
tree = ast.parse(inspect.getsource(gateway_run))
|
||||
|
||||
found = False
|
||||
for node in ast.walk(tree):
|
||||
if not isinstance(node, ast.If):
|
||||
continue
|
||||
calls = _calls(node)
|
||||
if (
|
||||
"_set_session_reasoning_override" in calls
|
||||
and _assigns_false(node, "was_auto_reset")
|
||||
):
|
||||
assert _references_name(node, "_last_resolved_model") and "pop" in _calls(
|
||||
node
|
||||
), (
|
||||
"gateway/run.py auto-reset cleanup block must also pop the "
|
||||
"session's entry from `_last_resolved_model`, mirroring the "
|
||||
"existing `_session_model_overrides`/`_pending_model_notes` "
|
||||
"pops in the same block (#58403)."
|
||||
)
|
||||
found = True
|
||||
break
|
||||
assert found, (
|
||||
"could not locate the auto-reset transient-state cleanup block in "
|
||||
"gateway/run.py (fingerprint: _set_session_reasoning_override + "
|
||||
"was_auto_reset = False)."
|
||||
)
|
||||
|
|
|
|||
|
|
@ -211,6 +211,35 @@ class TestHandleResumeCommand:
|
|||
assert runner._pending_model_notes["agent:main:telegram:dm:other"] == "[Note: keep-me]"
|
||||
db.close()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_resume_clears_last_resolved_model(self, tmp_path):
|
||||
"""Resume must also clear the resumed chat's cached last-resolved
|
||||
model, so the restored conversation re-resolves from current config
|
||||
instead of a value cached before the switch (mirrors /new and the
|
||||
compression-exhausted auto-reset, #58403), while leaving other
|
||||
chats' cache entries intact."""
|
||||
from hermes_state import SessionDB
|
||||
db = SessionDB(db_path=tmp_path / "state.db")
|
||||
db.create_session("old_session_abc", "telegram", user_id="12345", chat_id="67890")
|
||||
db.set_session_title("old_session_abc", "My Project")
|
||||
db.create_session("current_session_001", "telegram", user_id="12345", chat_id="67890")
|
||||
|
||||
event = _make_event(text="/resume My Project")
|
||||
runner = _make_runner(session_db=db, current_session_id="current_session_001",
|
||||
event=event)
|
||||
key = _session_key_for_event(event)
|
||||
runner._last_resolved_model = {
|
||||
key: "gpt-5",
|
||||
"agent:main:telegram:dm:other": "keep-me",
|
||||
}
|
||||
|
||||
result = await runner._handle_resume_command(event)
|
||||
|
||||
assert "Resumed" in result
|
||||
assert key not in runner._last_resolved_model
|
||||
assert runner._last_resolved_model["agent:main:telegram:dm:other"] == "keep-me"
|
||||
db.close()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_resume_nonexistent_name(self, tmp_path):
|
||||
"""Returns error for unknown session name."""
|
||||
|
|
|
|||
|
|
@ -253,3 +253,81 @@ async def test_idle_expiry_fires_finalize_hook(mock_invoke_hook):
|
|||
f"on_session_finalize was not fired during idle expiry; "
|
||||
f"got session_ids={session_ids} (regression of #14981)"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("hermes_cli.plugins.invoke_hook")
|
||||
async def test_idle_expiry_clears_last_resolved_model(mock_invoke_hook):
|
||||
"""Regression test for #58403.
|
||||
|
||||
``_session_expiry_watcher`` permanently finalizes an expired session and
|
||||
already drops ``_session_model_overrides`` / the reasoning override /
|
||||
``_pending_model_notes`` — a resumed conversation must not inherit stale
|
||||
per-session state. It missed ``_last_resolved_model``: without clearing
|
||||
it, a resumed session could serve a cached model from before it went
|
||||
idle on a transient config-cache miss, exactly the #58403 class the
|
||||
/new and compression-exhausted-reset paths already guard against.
|
||||
"""
|
||||
from datetime import datetime, timedelta
|
||||
|
||||
from gateway.run import GatewayRunner
|
||||
|
||||
runner = object.__new__(GatewayRunner)
|
||||
runner._running = True
|
||||
runner._running_agents = {}
|
||||
runner._agent_cache = {}
|
||||
runner._agent_cache_lock = None
|
||||
runner._last_session_store_prune_ts = 0.0
|
||||
|
||||
session_key = "agent:main:telegram:dm:42"
|
||||
expired_entry = SessionEntry(
|
||||
session_key=session_key,
|
||||
session_id="sess-expired",
|
||||
created_at=datetime.now() - timedelta(hours=2),
|
||||
updated_at=datetime.now() - timedelta(hours=2),
|
||||
platform=Platform.TELEGRAM,
|
||||
chat_type="dm",
|
||||
)
|
||||
expired_entry.expiry_finalized = False
|
||||
|
||||
runner.session_store = MagicMock()
|
||||
runner.session_store._ensure_loaded = MagicMock()
|
||||
runner.session_store._entries = {session_key: expired_entry}
|
||||
runner.session_store._is_session_expired = MagicMock(return_value=True)
|
||||
runner.session_store._lock = MagicMock()
|
||||
runner.session_store._lock.__enter__ = MagicMock(return_value=None)
|
||||
runner.session_store._lock.__exit__ = MagicMock(return_value=None)
|
||||
runner.session_store._save = MagicMock()
|
||||
|
||||
runner._evict_cached_agent = MagicMock()
|
||||
runner._cleanup_agent_resources = MagicMock()
|
||||
runner._sweep_idle_cached_agents = MagicMock(return_value=0)
|
||||
runner._session_model_overrides = {}
|
||||
runner._pending_model_notes = {}
|
||||
runner._last_resolved_model = {
|
||||
session_key: "gpt-5",
|
||||
"agent:main:telegram:dm:other": "keep-me",
|
||||
}
|
||||
|
||||
_orig_sleep = __import__("asyncio").sleep
|
||||
|
||||
async def _fast_sleep(_):
|
||||
await _orig_sleep(0)
|
||||
|
||||
def _hook_and_stop(*a, **kw):
|
||||
runner._running = False
|
||||
return None
|
||||
|
||||
mock_invoke_hook.side_effect = _hook_and_stop
|
||||
|
||||
with patch("gateway.run.asyncio.sleep", side_effect=_fast_sleep):
|
||||
await runner._session_expiry_watcher(interval=0)
|
||||
|
||||
assert session_key not in runner._last_resolved_model, (
|
||||
"session-expiry finalization did not clear the expired session's "
|
||||
"_last_resolved_model entry (#58403)"
|
||||
)
|
||||
assert runner._last_resolved_model["agent:main:telegram:dm:other"] == "keep-me", (
|
||||
"session-expiry finalization must only clear the expired session's "
|
||||
"own key, not unrelated sessions' cached entries"
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue