fix(honcho): extend saveMessages=false guard to shutdown() flush
Salvages #67559 — original gated sync_turn/on_memory_write/on_session_end but missed shutdown(), whose flush_all() still persisted on exit. hermes-sweeper review (salvageability=high) flagged this as the one gap. Guard sits after the worker-thread joins, not at the top: cleanup is independent of persistence, and a top-of-method return would leak _prefetch_thread/_sync_thread. Adds TestShutdown and clarifies the saveMessages=false README row. Credit @Matroskin86 (original PR author).
This commit is contained in:
@@ -215,7 +215,7 @@ Pick **[e]** at the prompt to set the three keys directly instead of going throu
|
||||
| Key | Type | Default | Description |
|
||||
|-----|------|---------|-------------|
|
||||
| `writeFrequency` | string/int | `"async"` | `"async"` (background), `"turn"` (sync per turn), `"session"` (batch on end), or integer N (every N turns) |
|
||||
| `saveMessages` | bool | `true` | Persist messages to Honcho API |
|
||||
| `saveMessages` | bool | `true` | Persist messages to Honcho API. When `false`, all automatic writes are skipped — raw turns (`sync_turn`), conclusion mirroring (`on_memory_write`), and session-end/shutdown flushes — while read and tools paths stay fully functional. |
|
||||
|
||||
### Session Resolution
|
||||
|
||||
|
||||
@@ -1665,7 +1665,12 @@ class HonchoMemoryProvider(MemoryProvider):
|
||||
for t in (self._prefetch_thread, self._sync_thread):
|
||||
if t and t.is_alive():
|
||||
t.join(timeout=5.0)
|
||||
# Flush any remaining messages
|
||||
# Flush any remaining messages. Honors saveMessages: false — skip
|
||||
# persistence, but the worker-thread joins above still run (cleanup
|
||||
# is independent of persistence; placing the guard here rather than
|
||||
# at the top avoids leaking _prefetch_thread/_sync_thread).
|
||||
if not getattr(self._config, "save_messages", True):
|
||||
return
|
||||
if self._manager and not (self._init_thread and self._init_thread.is_alive() and not self._session_initialized):
|
||||
try:
|
||||
self._manager.flush_all()
|
||||
|
||||
@@ -63,3 +63,27 @@ class TestOnSessionEnd:
|
||||
p = _provider(save_messages=True)
|
||||
p.on_session_end([])
|
||||
p._manager.flush_all.assert_called_once()
|
||||
|
||||
|
||||
class TestShutdown:
|
||||
"""shutdown() joins worker threads then flushes; saveMessages=false must
|
||||
skip the flush (persistence) while still running the joins (cleanup)."""
|
||||
|
||||
def _provider_for_shutdown(self, save_messages: bool) -> HonchoMemoryProvider:
|
||||
p = _provider(save_messages=save_messages)
|
||||
# shutdown() iterates these thread handles; if no turn/session-end ran
|
||||
# they may be unset, so default to None (= "no thread started").
|
||||
p._init_thread = None
|
||||
p._prefetch_thread = None
|
||||
p._sync_thread = None
|
||||
return p
|
||||
|
||||
def test_disabled_skips_flush(self):
|
||||
p = self._provider_for_shutdown(save_messages=False)
|
||||
p.shutdown()
|
||||
p._manager.flush_all.assert_not_called()
|
||||
|
||||
def test_enabled_flushes(self):
|
||||
p = self._provider_for_shutdown(save_messages=True)
|
||||
p.shutdown()
|
||||
p._manager.flush_all.assert_called_once()
|
||||
|
||||
Reference in New Issue
Block a user