From 2d35016b94a9c7cad718a43fd5610933f5e45f97 Mon Sep 17 00:00:00 2001 From: Erosika Date: Wed, 11 Mar 2026 18:21:27 -0400 Subject: [PATCH] fix(honcho): harden tool gating and migration peer routing Prevent stale Honcho tool exposure in context/local modes, restore reliable async write retry behavior, and ensure SOUL.md migration uploads target the AI peer instead of the user peer. Also align Honcho CLI key checks with host-scoped apiKey resolution and lock the fixes with regression tests. Made-with: Cursor --- honcho_integration/cli.py | 10 +- honcho_integration/session.py | 123 ++++++++++++------ run_agent.py | 53 ++++++-- tests/honcho_integration/test_async_memory.py | 62 ++++++++- tests/honcho_integration/test_cli.py | 29 +++++ tests/test_run_agent.py | 76 +++++++++++ 6 files changed, 297 insertions(+), 56 deletions(-) create mode 100644 tests/honcho_integration/test_cli.py diff --git a/honcho_integration/cli.py b/honcho_integration/cli.py index c899f9ff..ad4907c2 100644 --- a/honcho_integration/cli.py +++ b/honcho_integration/cli.py @@ -31,6 +31,12 @@ def _write_config(cfg: dict) -> None: ) +def _resolve_api_key(cfg: dict) -> str: + """Resolve API key with host -> root -> env fallback.""" + host_key = ((cfg.get("hosts") or {}).get(HOST) or {}).get("apiKey") + return host_key or cfg.get("apiKey", "") or os.environ.get("HONCHO_API_KEY", "") + + def _prompt(label: str, default: str | None = None, secret: bool = False) -> str: suffix = f" [{default}]" if default else "" sys.stdout.write(f" {label}{suffix}: ") @@ -435,7 +441,7 @@ def cmd_tokens(args) -> None: def cmd_identity(args) -> None: """Seed AI peer identity or show both peer representations.""" cfg = _read_config() - if not cfg.get("apiKey"): + if not _resolve_api_key(cfg): print(" No API key configured. Run 'hermes honcho setup' first.\n") return @@ -533,7 +539,7 @@ def cmd_migrate(args) -> None: agent_files.append(p) cfg = _read_config() - has_key = bool(cfg.get("apiKey", "")) + has_key = bool(_resolve_api_key(cfg)) print("\nHoncho migration: OpenClaw native memory → Hermes\n" + "─" * 50) print() diff --git a/honcho_integration/session.py b/honcho_integration/session.py index 19c41989..3d06d2f7 100644 --- a/honcho_integration/session.py +++ b/honcho_integration/session.py @@ -270,10 +270,10 @@ class HonchoSessionManager: self._cache[key] = session return session - def _flush_session(self, session: HonchoSession) -> None: + def _flush_session(self, session: HonchoSession) -> bool: """Internal: write unsynced messages to Honcho synchronously.""" if not session.messages: - return + return True user_peer = self._get_or_create_peer(session.user_peer_id) assistant_peer = self._get_or_create_peer(session.assistant_peer_id) @@ -286,7 +286,7 @@ class HonchoSessionManager: new_messages = [m for m in session.messages if not m.get("_synced")] if not new_messages: - return + return True honcho_messages = [] for msg in new_messages: @@ -298,12 +298,14 @@ class HonchoSessionManager: for msg in new_messages: msg["_synced"] = True logger.debug("Synced %d messages to Honcho for %s", len(honcho_messages), session.key) + self._cache[session.key] = session + return True except Exception as e: for msg in new_messages: msg["_synced"] = False logger.error("Failed to sync messages to Honcho: %s", e) - - self._cache[session.key] = session + self._cache[session.key] = session + return False def _async_writer_loop(self) -> None: """Background daemon thread: drains the async write queue.""" @@ -312,16 +314,33 @@ class HonchoSessionManager: item = self._async_queue.get(timeout=5) if item is _ASYNC_SHUTDOWN: break + + first_error: Exception | None = None try: - self._flush_session(item) + success = self._flush_session(item) except Exception as e: - logger.warning("Honcho async write failed, retrying once: %s", e) - import time as _time - _time.sleep(2) - try: - self._flush_session(item) - except Exception as e2: - logger.error("Honcho async write retry failed, dropping batch: %s", e2) + success = False + first_error = e + + if success: + continue + + if first_error is not None: + logger.warning("Honcho async write failed, retrying once: %s", first_error) + else: + logger.warning("Honcho async write failed, retrying once") + + import time as _time + _time.sleep(2) + + try: + retry_success = self._flush_session(item) + except Exception as e2: + logger.error("Honcho async write retry failed, dropping batch: %s", e2) + continue + + if not retry_success: + logger.error("Honcho async write retry failed, dropping batch") except queue.Empty: continue except Exception as e: @@ -617,21 +636,17 @@ class HonchoSessionManager: Returns: True if upload succeeded, False otherwise. """ - sanitized = self._sanitize_id(session_key) - honcho_session = self._sessions_cache.get(sanitized) + session = self._cache.get(session_key) + if not session: + logger.warning("No local session cached for '%s', skipping migration", session_key) + return False + + honcho_session = self._sessions_cache.get(session.honcho_session_id) if not honcho_session: logger.warning("No Honcho session cached for '%s', skipping migration", session_key) return False - # Resolve user peer for attribution - parts = session_key.split(":", 1) - channel = parts[0] if len(parts) > 1 else "default" - chat_id = parts[1] if len(parts) > 1 else session_key - user_peer_id = self._sanitize_id(f"user-{channel}-{chat_id}") - user_peer = self._peers_cache.get(user_peer_id) - if not user_peer: - logger.warning("No user peer cached for '%s', skipping migration", user_peer_id) - return False + user_peer = self._get_or_create_peer(session.user_peer_id) content_bytes = self._format_migration_transcript(session_key, messages) first_ts = messages[0].get("timestamp") if messages else None @@ -700,30 +715,45 @@ class HonchoSessionManager: if not memory_path.exists(): return False - sanitized = self._sanitize_id(session_key) - honcho_session = self._sessions_cache.get(sanitized) + session = self._cache.get(session_key) + if not session: + logger.warning("No local session cached for '%s', skipping memory migration", session_key) + return False + + honcho_session = self._sessions_cache.get(session.honcho_session_id) if not honcho_session: logger.warning("No Honcho session cached for '%s', skipping memory migration", session_key) return False - # Resolve user peer for attribution - parts = session_key.split(":", 1) - channel = parts[0] if len(parts) > 1 else "default" - chat_id = parts[1] if len(parts) > 1 else session_key - user_peer_id = self._sanitize_id(f"user-{channel}-{chat_id}") - user_peer = self._peers_cache.get(user_peer_id) - if not user_peer: - logger.warning("No user peer cached for '%s', skipping memory migration", user_peer_id) - return False + user_peer = self._get_or_create_peer(session.user_peer_id) + assistant_peer = self._get_or_create_peer(session.assistant_peer_id) uploaded = False files = [ - ("MEMORY.md", "consolidated_memory.md", "Long-term agent notes and preferences"), - ("USER.md", "user_profile.md", "User profile and preferences"), - ("SOUL.md", "agent_soul.md", "Agent persona and identity configuration"), + ( + "MEMORY.md", + "consolidated_memory.md", + "Long-term agent notes and preferences", + user_peer, + "user", + ), + ( + "USER.md", + "user_profile.md", + "User profile and preferences", + user_peer, + "user", + ), + ( + "SOUL.md", + "agent_soul.md", + "Agent persona and identity configuration", + assistant_peer, + "ai", + ), ] - for filename, upload_name, description in files: + for filename, upload_name, description, target_peer, target_kind in files: filepath = memory_path / filename if not filepath.exists(): continue @@ -745,10 +775,19 @@ class HonchoSessionManager: try: honcho_session.upload_file( file=(upload_name, wrapped.encode("utf-8"), "text/plain"), - peer=user_peer, - metadata={"source": "local_memory", "original_file": filename}, + peer=target_peer, + metadata={ + "source": "local_memory", + "original_file": filename, + "target_peer": target_kind, + }, + ) + logger.info( + "Uploaded %s to Honcho for %s (%s peer)", + filename, + session_key, + target_kind, ) - logger.info("Uploaded %s to Honcho for %s", filename, session_key) uploaded = True except Exception as e: logger.error("Failed to upload %s to Honcho: %s", filename, e) diff --git a/run_agent.py b/run_agent.py index ab27efbb..3bf7e4e2 100644 --- a/run_agent.py +++ b/run_agent.py @@ -100,6 +100,13 @@ from agent.trajectory import ( save_trajectory as _save_trajectory_to_file, ) +HONCHO_TOOL_NAMES = { + "honcho_context", + "honcho_profile", + "honcho_search", + "honcho_conclude", +} + class IterationBudget: """Thread-safe shared iteration counter for parent and child agents. @@ -607,6 +614,11 @@ class AIAgent: print(" Run 'hermes honcho setup' to reconfigure.") self._honcho = None + # Tools are initially discovered before Honcho activation. If Honcho + # stays inactive, remove any stale honcho_* tools from prior process state. + if not self._honcho: + self._strip_honcho_tools_from_surface() + # Gate local memory writes based on per-peer memory modes. # AI peer governs MEMORY.md; user peer governs USER.md. # "honcho" = Honcho only, disable local; "local" = local only, no Honcho sync. @@ -1342,6 +1354,20 @@ class AIAgent: for peer in (hcfg.ai_peer, hcfg.peer_name or "user") ) + def _strip_honcho_tools_from_surface(self) -> None: + """Remove Honcho tools from the active tool surface.""" + if not self.tools: + self.valid_tool_names = set() + return + + self.tools = [ + tool for tool in self.tools + if tool.get("function", {}).get("name") not in HONCHO_TOOL_NAMES + ] + self.valid_tool_names = { + tool["function"]["name"] for tool in self.tools + } if self.tools else set() + def _activate_honcho( self, hcfg, @@ -1386,19 +1412,24 @@ class AIAgent: set_session_context(self._honcho, self._honcho_session_key) - if hcfg.recall_mode != "context": - self.tools = get_tool_definitions( - enabled_toolsets=enabled_toolsets, - disabled_toolsets=disabled_toolsets, - quiet_mode=True, - ) - self.valid_tool_names = { - tool["function"]["name"] for tool in self.tools - } if self.tools else set() + # Rebuild tool surface after Honcho context injection. Tool availability + # is check_fn-gated and may change once session context is attached. + self.tools = get_tool_definitions( + enabled_toolsets=enabled_toolsets, + disabled_toolsets=disabled_toolsets, + quiet_mode=True, + ) + self.valid_tool_names = { + tool["function"]["name"] for tool in self.tools + } if self.tools else set() + + if hcfg.recall_mode == "context": + self._strip_honcho_tools_from_surface() + if not self.quiet_mode: + print(" Honcho active — recall_mode: context (tools suppressed)") + else: if not self.quiet_mode: print(f" Honcho active — recall_mode: {hcfg.recall_mode}") - elif not self.quiet_mode: - print(" Honcho active — recall_mode: context (tools suppressed)") logger.info( "Honcho active (session: %s, user: %s, workspace: %s, " diff --git a/tests/honcho_integration/test_async_memory.py b/tests/honcho_integration/test_async_memory.py index 52a03ac2..908c0fc6 100644 --- a/tests/honcho_integration/test_async_memory.py +++ b/tests/honcho_integration/test_async_memory.py @@ -380,10 +380,10 @@ class TestAsyncWriterThread: sess.add_message("user", "async msg") flushed = [] - original = mgr._flush_session def capture(s): flushed.append(s) + return True mgr._flush_session = capture mgr._async_queue.put(sess) @@ -457,6 +457,66 @@ class TestAsyncWriterRetry: assert call_count[0] == 2 assert not mgr._async_thread.is_alive() + def test_retries_when_flush_reports_failure(self): + mgr = _make_manager(write_frequency="async") + sess = _make_session() + sess.add_message("user", "msg") + + call_count = [0] + + def fail_then_succeed(_session): + call_count[0] += 1 + return call_count[0] > 1 + + mgr._flush_session = fail_then_succeed + + with patch("time.sleep"): + mgr._async_queue.put(sess) + deadline = time.time() + 3.0 + while call_count[0] < 2 and time.time() < deadline: + time.sleep(0.05) + + mgr.shutdown() + assert call_count[0] == 2 + + +class TestMemoryFileMigrationTargets: + def test_soul_upload_targets_ai_peer(self, tmp_path): + mgr = _make_manager(write_frequency="turn") + session = _make_session( + key="cli:test", + user_peer_id="custom-user", + assistant_peer_id="custom-ai", + honcho_session_id="cli-test", + ) + mgr._cache[session.key] = session + + user_peer = MagicMock(name="user-peer") + ai_peer = MagicMock(name="ai-peer") + mgr._peers_cache[session.user_peer_id] = user_peer + mgr._peers_cache[session.assistant_peer_id] = ai_peer + + honcho_session = MagicMock() + mgr._sessions_cache[session.honcho_session_id] = honcho_session + + (tmp_path / "MEMORY.md").write_text("memory facts", encoding="utf-8") + (tmp_path / "USER.md").write_text("user profile", encoding="utf-8") + (tmp_path / "SOUL.md").write_text("ai identity", encoding="utf-8") + + uploaded = mgr.migrate_memory_files(session.key, str(tmp_path)) + + assert uploaded is True + assert honcho_session.upload_file.call_count == 3 + + peer_by_upload_name = {} + for call_args in honcho_session.upload_file.call_args_list: + payload = call_args.kwargs["file"] + peer_by_upload_name[payload[0]] = call_args.kwargs["peer"] + + assert peer_by_upload_name["consolidated_memory.md"] is user_peer + assert peer_by_upload_name["user_profile.md"] is user_peer + assert peer_by_upload_name["agent_soul.md"] is ai_peer + # --------------------------------------------------------------------------- # HonchoClientConfig dataclass defaults for new fields diff --git a/tests/honcho_integration/test_cli.py b/tests/honcho_integration/test_cli.py new file mode 100644 index 00000000..b5a1c9f6 --- /dev/null +++ b/tests/honcho_integration/test_cli.py @@ -0,0 +1,29 @@ +"""Tests for Honcho CLI helpers.""" + +from honcho_integration.cli import _resolve_api_key + + +class TestResolveApiKey: + def test_prefers_host_scoped_key(self): + cfg = { + "apiKey": "root-key", + "hosts": { + "hermes": { + "apiKey": "host-key", + } + }, + } + assert _resolve_api_key(cfg) == "host-key" + + def test_falls_back_to_root_key(self): + cfg = { + "apiKey": "root-key", + "hosts": {"hermes": {}}, + } + assert _resolve_api_key(cfg) == "root-key" + + def test_falls_back_to_env_key(self, monkeypatch): + monkeypatch.setenv("HONCHO_API_KEY", "env-key") + assert _resolve_api_key({}) == "env-key" + monkeypatch.delenv("HONCHO_API_KEY", raising=False) + diff --git a/tests/test_run_agent.py b/tests/test_run_agent.py index e29ef618..be6f6d51 100644 --- a/tests/test_run_agent.py +++ b/tests/test_run_agent.py @@ -1277,6 +1277,82 @@ class TestHonchoActivation: ) mock_client.assert_not_called() + def test_recall_mode_context_suppresses_honcho_tools(self): + hcfg = HonchoClientConfig( + enabled=True, + api_key="honcho-key", + memory_mode="hybrid", + peer_name="user", + ai_peer="hermes", + recall_mode="context", + ) + manager = MagicMock() + manager._config = hcfg + manager.get_or_create.return_value = SimpleNamespace(messages=[]) + manager.get_prefetch_context.return_value = {"representation": "Known user", "card": ""} + + with ( + patch( + "run_agent.get_tool_definitions", + side_effect=[ + _make_tool_defs("web_search"), + _make_tool_defs( + "web_search", + "honcho_context", + "honcho_profile", + "honcho_search", + "honcho_conclude", + ), + ], + ), + patch("run_agent.check_toolset_requirements", return_value={}), + patch("run_agent.OpenAI"), + patch("tools.honcho_tools.set_session_context"), + ): + agent = AIAgent( + api_key="test-key-1234567890", + quiet_mode=True, + skip_context_files=True, + skip_memory=False, + honcho_session_key="gateway-session", + honcho_manager=manager, + honcho_config=hcfg, + ) + + assert "web_search" in agent.valid_tool_names + assert "honcho_context" not in agent.valid_tool_names + assert "honcho_profile" not in agent.valid_tool_names + assert "honcho_search" not in agent.valid_tool_names + assert "honcho_conclude" not in agent.valid_tool_names + + def test_inactive_honcho_strips_stale_honcho_tools(self): + hcfg = HonchoClientConfig( + enabled=True, + api_key="honcho-key", + memory_mode="local", + peer_name="user", + ai_peer="hermes", + ) + + with ( + patch("run_agent.get_tool_definitions", return_value=_make_tool_defs("web_search", "honcho_context")), + patch("run_agent.check_toolset_requirements", return_value={}), + patch("run_agent.OpenAI"), + patch("honcho_integration.client.HonchoClientConfig.from_global_config", return_value=hcfg), + patch("honcho_integration.client.get_honcho_client") as mock_client, + ): + agent = AIAgent( + api_key="test-key-1234567890", + quiet_mode=True, + skip_context_files=True, + skip_memory=False, + ) + + assert agent._honcho is None + assert "web_search" in agent.valid_tool_names + assert "honcho_context" not in agent.valid_tool_names + mock_client.assert_not_called() + class TestHonchoPrefetchScheduling: def test_honcho_prefetch_includes_cached_dialectic(self, agent):