From d610b238c63ee865ad4e4ca8cf104f52720cff6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=B5=B5=E6=A1=82=E9=9B=84?= Date: Sat, 8 Aug 2026 00:54:29 +0800 Subject: [PATCH] fix(honcho): extend saveMessages=false guard to shutdown() flush MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- plugins/memory/honcho/README.md | 2 +- plugins/memory/honcho/__init__.py | 7 ++++++- tests/honcho_plugin/test_save_messages.py | 24 +++++++++++++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/plugins/memory/honcho/README.md b/plugins/memory/honcho/README.md index e7d41d1cf5..0c849a9a33 100644 --- a/plugins/memory/honcho/README.md +++ b/plugins/memory/honcho/README.md @@ -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 diff --git a/plugins/memory/honcho/__init__.py b/plugins/memory/honcho/__init__.py index 02c4aa84ed..f5bebd195c 100644 --- a/plugins/memory/honcho/__init__.py +++ b/plugins/memory/honcho/__init__.py @@ -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() diff --git a/tests/honcho_plugin/test_save_messages.py b/tests/honcho_plugin/test_save_messages.py index 017b2eaada..afbe546606 100644 --- a/tests/honcho_plugin/test_save_messages.py +++ b/tests/honcho_plugin/test_save_messages.py @@ -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()