mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-07-18 13:40:22 +00:00
fix(streaming): degrade self-heal SessionDB to None on failed rebuild
Codex gate on #6143 found a SILENT regression the reuse-path fix introduced: _replace_session_db_in_kwargs early-returned the OLD handle when the rebuild failed, but master assigned None. If the old handle was already CLOSED, both credential self-heal callers (streaming.py:9032/10253) then built a fresh agent around a closed SessionDB -> every persist/search fails with 'NoneType has no attribute execute' while the chat keeps going. Guard on _session_db_is_open: keep a live handle (subagents hold it by ref), else degrade to None (clean lazy reinit, master behaviour). +2 regression tests. Co-authored-by: carlotestor <carlotestor@users.noreply.github.com>
This commit is contained in:
+10
-1
@@ -6350,7 +6350,16 @@ def _replace_session_db_in_kwargs(agent_kwargs, state_db_path):
|
||||
_old_session_db = agent_kwargs.get("session_db")
|
||||
_next_session_db = _build_session_db_for_stream(state_db_path)
|
||||
if _next_session_db is None:
|
||||
return _old_session_db
|
||||
# Replacement construction failed. Keep the prior handle only if it is
|
||||
# still open (live subagents may hold it by reference); otherwise
|
||||
# degrade cleanly to None — as master did — so the rebuilt agent lazily
|
||||
# reinitialises its SessionDB instead of reusing a closed handle and
|
||||
# failing every persist/search with
|
||||
# "'NoneType' object has no attribute 'execute'".
|
||||
if _session_db_is_open(_old_session_db):
|
||||
return _old_session_db
|
||||
agent_kwargs["session_db"] = None
|
||||
return None
|
||||
if _session_db_is_open(_old_session_db):
|
||||
# Keep the live handle; discard the unused new one.
|
||||
try:
|
||||
|
||||
@@ -280,3 +280,55 @@ def test_lru_eviction_closes_evicted_session_db():
|
||||
assert db1.close_calls == 1, "evicted agent's SessionDB must be closed exactly once"
|
||||
assert db2.close_calls == 0, "remaining agents' SessionDBs must not be touched"
|
||||
assert db3.close_calls == 0
|
||||
|
||||
|
||||
# ── 6: self-heal path must not reuse a CLOSED handle when the rebuild fails ──
|
||||
|
||||
|
||||
def _import_replace_helper():
|
||||
"""Import the real credential-self-heal SessionDB replacer."""
|
||||
try:
|
||||
from api.streaming import _replace_session_db_in_kwargs # type: ignore
|
||||
except Exception as exc:
|
||||
pytest.skip(f"api.streaming not importable: {exc}")
|
||||
return _replace_session_db_in_kwargs
|
||||
|
||||
|
||||
def test_replace_degrades_to_none_when_rebuild_fails_and_old_is_closed(monkeypatch):
|
||||
"""Credential self-heal regression (Codex gate finding on PR #6143).
|
||||
|
||||
When ``_build_session_db_for_stream`` returns None (rebuild failed) AND the
|
||||
prior handle is already CLOSED, ``_replace_session_db_in_kwargs`` must leave
|
||||
``agent_kwargs['session_db'] = None`` — as master did — so the rebuilt agent
|
||||
lazily reinitialises. Retaining the closed handle (the pre-fix behaviour)
|
||||
makes every persist/search fail with
|
||||
``'NoneType' object has no attribute 'execute'`` while the chat continues.
|
||||
"""
|
||||
import api.streaming as streaming
|
||||
|
||||
_replace = _import_replace_helper()
|
||||
monkeypatch.setattr(streaming, "_build_session_db_for_stream", lambda _p: None)
|
||||
|
||||
old_db = _MockSessionDB("old", open_=False) # already closed
|
||||
kwargs = {"session_db": old_db}
|
||||
result = _replace(kwargs, "/tmp/does-not-matter.db")
|
||||
|
||||
assert result is None, "must not hand back a closed handle when rebuild fails"
|
||||
assert kwargs["session_db"] is None, "kwargs must degrade to None (clean lazy reinit)"
|
||||
|
||||
|
||||
def test_replace_keeps_open_handle_when_rebuild_fails(monkeypatch):
|
||||
"""Inverse: a still-OPEN prior handle (held by live subagents) is retained
|
||||
when the rebuild fails — do not orphan a live shared connection."""
|
||||
import api.streaming as streaming
|
||||
|
||||
_replace = _import_replace_helper()
|
||||
monkeypatch.setattr(streaming, "_build_session_db_for_stream", lambda _p: None)
|
||||
|
||||
old_db = _MockSessionDB("old", open_=True) # still live (subagents hold it)
|
||||
kwargs = {"session_db": old_db}
|
||||
result = _replace(kwargs, "/tmp/does-not-matter.db")
|
||||
|
||||
assert result is old_db, "a live handle must be kept when the rebuild fails"
|
||||
assert kwargs["session_db"] is old_db
|
||||
assert old_db.close_calls == 0, "must not close a live shared handle"
|
||||
|
||||
Reference in New Issue
Block a user