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
This commit is contained in:
parent
8cddcfa0d8
commit
2d35016b94
@ -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()
|
||||
|
||||
@ -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)
|
||||
|
||||
53
run_agent.py
53
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, "
|
||||
|
||||
@ -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
|
||||
|
||||
29
tests/honcho_integration/test_cli.py
Normal file
29
tests/honcho_integration/test_cli.py
Normal file
@ -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)
|
||||
|
||||
@ -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):
|
||||
|
||||
Loading…
Reference in New Issue
Block a user