From ffdc0b0be6e0abed164bbfe1bdf95ba0959f64d1 Mon Sep 17 00:00:00 2001 From: Dillon Townsel Date: Sat, 8 Aug 2026 19:47:07 -0500 Subject: [PATCH] fix(honcho): enforce saveMessages write containment + reject gateway-internal turns --- plugins/memory/honcho/__init__.py | 39 +++++ tests/test_honcho_startup_fail_open.py | 224 +++++++++++++++++++++++++ 2 files changed, 263 insertions(+) diff --git a/plugins/memory/honcho/__init__.py b/plugins/memory/honcho/__init__.py index 331950e696..913d3bd619 100644 --- a/plugins/memory/honcho/__init__.py +++ b/plugins/memory/honcho/__init__.py @@ -30,6 +30,30 @@ from tools.registry import tool_error logger = logging.getLogger(__name__) +# Gateway-internal notifications can arrive through the same user-role channel +# as genuine user messages. They are execution metadata, not conversation, and +# must never become durable personal memory. Keep this deliberately anchored: +# a human discussing one of these strings mid-message is still valid input. +_INTERNAL_GATEWAY_TURN_RE = re.compile( + r"^\s*(?:" + r"\[ASYNC (?:DELEGATION )?(?:BATCH )?COMPLETE[^\]]*\]|" + r"\[CONTEXT COMPACTION[^\]]*\]|" + r"\[CONTEXT SUMMARY\]:?|" + r"\[PRIOR CONTEXT[^\]]*\]|" + r"\[Your active task list was preserved across context compression\]|" + r"\[IMPORTANT: Background process \d+ matched watch pattern[^\n]*|" + r"A background fan-out of \d+ subagent\(s\) you dispatched earlier has finished\.|" + r"A background subagent you dispatched earlier has finished\." + r")", + re.IGNORECASE, +) + + +def _is_internal_gateway_turn(text: str) -> bool: + """Return True for machine-generated gateway/delegation notifications.""" + return bool(_INTERNAL_GATEWAY_TURN_RE.match(text or "")) + + # --------------------------------------------------------------------------- # Tool schemas (moved from tools/honcho_tools.py) # --------------------------------------------------------------------------- @@ -1388,6 +1412,14 @@ class HonchoMemoryProvider(MemoryProvider): """ if self._cron_skipped: return + # ``saveMessages`` is the operator's hard write gate. Previously it + # was parsed into HonchoClientConfig but never enforced here, so a + # cached hybrid provider kept writing even after containment was set. + if self._config and not getattr(self._config, "save_messages", True): + return + if _is_internal_gateway_turn(user_content): + logger.debug("Honcho sync skipped machine-generated gateway turn") + return if self._recall_mode == "tools" and not self._session_ready(): return if not self._session_ready(): @@ -1397,6 +1429,8 @@ class HonchoMemoryProvider(MemoryProvider): msg_limit = self._config.message_max_chars if self._config else 25000 clean_user_content = sanitize_context(user_content or "").strip() clean_assistant_content = sanitize_context(assistant_content or "").strip() + if not clean_user_content or not clean_assistant_content: + return def _sync(): try: @@ -1432,6 +1466,11 @@ class HonchoMemoryProvider(MemoryProvider): return if self._cron_skipped: return + # ``saveMessages`` is the operator's hard write gate; the memory-tool + # mirror is an automatic Honcho mutation path and must respect it too, + # otherwise containment would only cover conversation turns. + if self._config and not getattr(self._config, "save_messages", True): + return if self._recall_mode == "tools" and not self._session_ready(): return if not self._session_ready(): diff --git a/tests/test_honcho_startup_fail_open.py b/tests/test_honcho_startup_fail_open.py index 3d71ef83f6..09daabb393 100644 --- a/tests/test_honcho_startup_fail_open.py +++ b/tests/test_honcho_startup_fail_open.py @@ -324,3 +324,227 @@ def test_honcho_tools_lazy_hooks_do_not_prestart_background_init(monkeypatch): assert result == {"result": ["ready"]} assert init_calls == ["session-1"] assert not background_started.is_set() + + +# --------------------------------------------------------------------------- +# Write-containment regression tests +# --------------------------------------------------------------------------- + + +def test_honcho_sync_turn_skips_write_when_save_messages_is_disabled(): + """The resolved write-disable switch must gate an initialized provider.""" + provider = HonchoMemoryProvider() + cfg = _configured_tools_config(init_on_session_start=True) + cfg.save_messages = False + manager_calls = [] + + class Manager: + def get_or_create(self, session_key): + manager_calls.append(session_key) + return SimpleNamespace() + + provider._config = cfg + provider._manager = Manager() + provider._session_key = "test-session" + provider._session_initialized = True + + provider.sync_turn("a genuine user turn", "a genuine assistant reply") + + assert provider._sync_thread is None + assert manager_calls == [] + + +def test_honcho_sync_turn_skips_anchored_gateway_notifications(): + """Known bracketed gateway wrappers must not become durable messages.""" + wrappers = ( + "[ASYNC DELEGATION BATCH COMPLETE — deleg_1]\nworker results follow", + "[ASYNC DELEGATION COMPLETE — deleg_2]", + "[CONTEXT COMPACTION — REFERENCE ONLY]\nsummary follows", + "[CONTEXT COMPACTION - REFERENCE ONLY]", + "[CONTEXT COMPACTION]", + "[PRIOR CONTEXT — for reference only; not a new message]", + "[Your active task list was preserved across context compression]", + "[CONTEXT SUMMARY]: previous context", + "[IMPORTANT: Background process 12 matched watch pattern \"foo\"\nCommand: x", + ) + + for wrapper in wrappers: + provider = HonchoMemoryProvider() + manager_calls = [] + + class Manager: + def get_or_create(self, session_key): + manager_calls.append(session_key) + return SimpleNamespace() + + provider._config = _configured_tools_config(init_on_session_start=True) + provider._manager = Manager() + provider._session_key = "test-session" + provider._session_initialized = True + + provider.sync_turn(wrapper, "assistant reply") + + assert provider._sync_thread is None, f"wrapper not suppressed: {wrapper[:60]!r}" + assert manager_calls == [], f"wrapper not suppressed: {wrapper[:60]!r}" + + +def test_honcho_sync_turn_skips_prose_gateway_notifications(): + """Prose-form gateway notifications must not become durable messages.""" + prose_wrappers = ( + "A background fan-out of 3 subagent(s) you dispatched earlier has finished.", + "A background subagent you dispatched earlier has finished. You may have moved on.", + ) + + for wrapper in prose_wrappers: + provider = HonchoMemoryProvider() + manager_calls = [] + + class Manager: + def get_or_create(self, session_key): + manager_calls.append(session_key) + return SimpleNamespace() + + provider._config = _configured_tools_config(init_on_session_start=True) + provider._manager = Manager() + provider._session_key = "test-session" + provider._session_initialized = True + + provider.sync_turn(wrapper, "assistant reply") + + assert provider._sync_thread is None, f"prose wrapper not suppressed: {wrapper[:60]!r}" + assert manager_calls == [], f"prose wrapper not suppressed: {wrapper[:60]!r}" + + +def test_honcho_sync_turn_does_not_suppress_genuine_user_messages(): + """Genuine user messages that mention gateway terms must still be stored.""" + genuine = ( + "A background process I ran has finished — can you check the output?", + "A background subagent you dispatched earlier has finished? no wait, I was asking about the report", + "the async delegation batch complete marker disappeared from my log", + "CONTEXT COMPACTION happened mid-message and I want to see it", + "When you see PRIOR CONTEXT, treat it carefully", + "I want to know about your task list", + "IMPORTANT: Background process — can you explain what that means?", + "[IMPORTANT: Background process — what does that mean?]", + ) + + for msg in genuine: + provider = HonchoMemoryProvider() + manager_calls = [] + + class Manager: + def get_or_create(self, session_key): + manager_calls.append(session_key) + return SimpleNamespace() + + provider._config = _configured_tools_config(init_on_session_start=True) + provider._manager = Manager() + provider._session_key = "test-session" + provider._session_initialized = True + + provider.sync_turn(msg, "assistant reply") + + assert provider._sync_thread is not None, f"genuine message suppressed: {msg[:60]!r}" + assert manager_calls != [], f"genuine message suppressed: {msg[:60]!r}" + + +def test_honcho_sync_turn_skips_empty_content(): + """Empty or whitespace-only turns must not be stored.""" + provider = HonchoMemoryProvider() + manager_calls = [] + + class Manager: + def get_or_create(self, session_key): + manager_calls.append(session_key) + return SimpleNamespace() + + provider._config = _configured_tools_config(init_on_session_start=True) + provider._manager = Manager() + provider._session_key = "test-session" + provider._session_initialized = True + + provider.sync_turn(" ", " ") + + assert provider._sync_thread is None + assert manager_calls == [] + + +def test_honcho_sync_turn_same_instance_config_flip_gates_writes(): + """The cached-provider regression: flipping save_messages on the SAME + configured instance must stop writes without re-initialization.""" + provider = HonchoMemoryProvider() + cfg = _configured_tools_config(init_on_session_start=True) + cfg.save_messages = True + manager_calls = [] + write_done = threading.Event() + + class Manager: + def get_or_create(self, session_key): + manager_calls.append(session_key) + return SimpleNamespace(add_message=lambda role, content: None) + + def _flush_session(self, session): + write_done.set() + + provider._config = cfg + provider._manager = Manager() + provider._session_key = "test-session" + provider._session_initialized = True + + # enabled -> write happens + provider.sync_turn("user turn", "assistant reply") + assert write_done.wait(timeout=5), "first write never completed" + + # operator flips containment on the same cached config object + cfg.save_messages = False + manager_calls.clear() + provider.sync_turn("user turn two", "assistant reply two") + + # no new write may occur; the stale _sync_thread from the enabled write is fine + assert manager_calls == [] + + +def test_honcho_on_memory_write_honors_save_messages_false(): + """The memory-tool mirror is an automatic write path and must respect the + write-disable switch; otherwise containment only covers conversation turns.""" + provider = HonchoMemoryProvider() + cfg = _configured_tools_config(init_on_session_start=True) + cfg.save_messages = False + conclusion_calls = [] + + class Manager: + def create_conclusion(self, session_key, content): + conclusion_calls.append((session_key, content)) + + provider._config = cfg + provider._manager = Manager() + provider._session_key = "test-session" + provider._session_initialized = True + + provider.on_memory_write("add", "user", "prefers fail-open memory") + + assert conclusion_calls == [] + + +def test_honcho_on_memory_write_still_writes_when_enabled(): + """With save_messages enabled, the memory-tool mirror still writes.""" + provider = HonchoMemoryProvider() + cfg = _configured_tools_config(init_on_session_start=True) + cfg.save_messages = True + conclusion_calls = [] + write_done = threading.Event() + + class Manager: + def create_conclusion(self, session_key, content): + conclusion_calls.append((session_key, content)) + write_done.set() + + provider._config = cfg + provider._manager = Manager() + provider._session_key = "test-session" + provider._session_initialized = True + + provider.on_memory_write("add", "user", "prefers fail-open memory") + + assert write_done.wait(timeout=5), "memory mirror write never completed" + assert conclusion_calls != []