From 3da6a80d50429cf9a14f04e3b2838045d9ce2309 Mon Sep 17 00:00:00 2001 From: Erosika Date: Fri, 4 Sep 2026 15:31:18 -0400 Subject: [PATCH] fix(honcho): refuse to mint a user peer when no identity or peerName exists a desktop or cli session with no peerName in honcho.json and no gateway user id landed on a peer derived from the session key: user-default- for per-directory sessions, user-- for keyed ones. every directory got its own phantom peer, so the operator's turns and memory never reached their real peer and the injected representation went stale (#93326). _resolve_user_peer_id now raises HonchoPeerUnresolvedError instead of deriving a name. a peer is either the declared peerName or an identity the transport supplied. the provider records the failure, tells the model once that memory is off and which key to set, returns the same detail from tool calls, and stops retrying init because a missing config key does not heal mid-session. the memory-file migration gate loses its "no owner and no runtime identity" branch: that cohort no longer has a session to migrate into. whitespace-only peerName is treated as unset rather than sanitized to "--". --- plugins/memory/honcho/README.md | 4 +- plugins/memory/honcho/__init__.py | 27 ++++++++++--- plugins/memory/honcho/session_migration.py | 9 ++--- plugins/memory/honcho/session_peers.py | 16 ++++++-- tests/honcho_plugin/test_async_memory.py | 18 ++++----- tests/honcho_plugin/test_auth_recovery.py | 2 +- tests/honcho_plugin/test_pin_peer_name.py | 29 +++++++++++--- tests/test_honcho_startup_fail_open.py | 44 ++++++++++++++++++++++ 8 files changed, 116 insertions(+), 33 deletions(-) diff --git a/plugins/memory/honcho/README.md b/plugins/memory/honcho/README.md index 547c961104..e8cdb836f6 100644 --- a/plugins/memory/honcho/README.md +++ b/plugins/memory/honcho/README.md @@ -214,9 +214,11 @@ In gateway deployments (Telegram, Discord, Slack, etc.) each user arrives with a 4. runtimePeerPrefix + runtime_id → namespaced peer, with sha256 collision escalation 5. raw sanitized runtime_id → fallback peer 6. peerName → no runtime ID at all (CLI/TUI) -7. session-key fallback → no config either +7. neither → session init fails with a one-time notice; no peer is minted ``` +Step 7 used to derive a peer from the session key (`user-default-`). That put a desktop or CLI session with no `peerName` on a phantom peer per directory, so its turns and memory never reached the operator's own peer (#93326). Set `peerName` (`hermes honcho peer --user `) or run under a gateway that supplies a user ID. + **Why no `pinAiPeer`?** The AI peer is already pinned by construction — `aiPeer` is the only AI-side identity setting and the resolver never overrides it. Only the user-side peer has the runtime-vs-config tension that `pinUserPeer` resolves. **Host vs root semantics.** All three keys are accepted at both root and `hosts.` levels. Host-level wins. For maps and prefixes, host-level *replaces* the root value as a whole (not merge), so a host can intentionally own its identity universe or wipe it with `userPeerAliases: {}` / `runtimePeerPrefix: ""`. diff --git a/plugins/memory/honcho/__init__.py b/plugins/memory/honcho/__init__.py index 09ff47671a..9f1e69bd43 100644 --- a/plugins/memory/honcho/__init__.py +++ b/plugins/memory/honcho/__init__.py @@ -161,6 +161,9 @@ class HonchoMemoryProvider(DialecticMixin, MemoryProvider): # Init auth failures live here because the failed manager is discarded. self._init_auth_failure: Optional[str] = None self._init_auth_notice_emitted = False + # Set when no user peer could be named (no runtime identity, no peerName); init is not retried. + self._init_peer_failure: Optional[str] = None + self._init_peer_notice_emitted = False self._cron_skipped = False # cron and flush contexts disable the plugin entirely @property @@ -270,8 +273,9 @@ class HonchoMemoryProvider(DialecticMixin, MemoryProvider): def _run_session_init(self, label: str) -> bool: """Run _do_session_init with the deferred kwargs; on failure discard the manager - and (for auth failures) keep the detail for the one-time notice.""" + and (for auth or unresolved-peer failures) keep the detail for the one-time notice.""" from plugins.memory.honcho.session import HonchoAuthError + from plugins.memory.honcho.session_peers import HonchoPeerUnresolvedError init_kwargs = self._lazy_init_kwargs if init_kwargs is None: # another init path already consumed the deferred kwargs @@ -286,6 +290,10 @@ class HonchoMemoryProvider(DialecticMixin, MemoryProvider): # Keep the auth detail so the one-time notice survives the manager discard. self._init_auth_failure = str(e) detail = "authentication rejected" + elif isinstance(e, HonchoPeerUnresolvedError): + # A missing peerName does not heal mid-session, so drop the deferred kwargs and stop retrying. + self._init_peer_failure = str(e) + self._lazy_init_kwargs = self._lazy_init_session_id = None logger.warning("Honcho %s session init failed: %s", label, detail) return False self._lazy_init_kwargs = self._lazy_init_session_id = None @@ -555,8 +563,8 @@ class HonchoMemoryProvider(DialecticMixin, MemoryProvider): if first_turn_base_deadline is not None and self._init_thread is not None: self._init_thread.join(timeout=max(0.0, first_turn_base_deadline - time.monotonic())) if not self._session_ready(): - # A failed auth init still owes the user the one-time notice. - return self._log_injection("session-not-ready", self._pop_auth_notice()) + # A failed init still owes the user its one-time notice. + return self._log_injection("session-not-ready", self._pop_auth_notice() or self._pop_peer_notice()) # Trivial turns start no work, but may consume a ready pending result. if self._is_trivial_prompt(query): @@ -592,6 +600,14 @@ class HonchoMemoryProvider(DialecticMixin, MemoryProvider): "Tell the user (once) that Honcho memory is paused and that running 'hermes honcho setup' " "to re-authenticate will restore it.") + def _pop_peer_notice(self) -> str: + """One-time model-facing notice that no user peer could be named and memory is off.""" + if self._init_peer_failure is None or self._init_peer_notice_emitted: + return "" + self._init_peer_notice_emitted = True + return (f"[Honcho memory status] Honcho memory is off for this session. {self._init_peer_failure}\n" + "Tell the user (once) that Honcho memory is off until honcho.json names a user peer.") + def _truncate_to_budget(self, text: str) -> str: """Truncate text to the context_tokens budget (≈4 chars/token) at a word boundary.""" if not self._config or not self._config.context_tokens: @@ -969,8 +985,9 @@ class HonchoMemoryProvider(DialecticMixin, MemoryProvider): if self._init_thread and self._init_thread.is_alive(): return tool_error("Honcho session is still initializing; try again shortly.") if not self._ensure_session(): - return tool_error(f"Honcho memory authentication failed: {self._init_auth_failure}" - if self._init_auth_failure else "Honcho session could not be initialized.") + if self._init_auth_failure: + return tool_error(f"Honcho memory authentication failed: {self._init_auth_failure}") + return tool_error(self._init_peer_failure or "Honcho session could not be initialized.") if not self._manager or not self._session_key: return tool_error("Honcho is not active for this session.") if (handler := self._TOOL_HANDLERS.get(tool_name)) is None: diff --git a/plugins/memory/honcho/session_migration.py b/plugins/memory/honcho/session_migration.py index c706107a2d..2deb187bb1 100644 --- a/plugins/memory/honcho/session_migration.py +++ b/plugins/memory/honcho/session_migration.py @@ -35,13 +35,10 @@ class SessionMigrationMixin: # Owner-scoped: these files describe the install owner; uploading them under another # human's peer would make Honcho attribute the owner's facts to that person. The owner is - # the CONFIG peerName — never a re-resolution of the session's own peer (that would compare - # the triggering user to themselves). No declared owner: single-operator only when there is - # no runtime identity; with one, nobody can be proven to be the owner. + # the CONFIG peerName, never a re-resolution of the session's own peer (that would compare + # the triggering user to themselves). No declared owner: nobody can be proven to be the owner. owner_peer_id = self._declared_owner_peer_id() - session_is_owner = (session.user_peer_id == owner_peer_id if owner_peer_id is not None - else not self._runtime_user_ids()) - if not session_is_owner: + if owner_peer_id is None or session.user_peer_id != owner_peer_id: logger.info("Skipping memory-file migration: session user peer '%s' is not the declared owner (peerName=%s)", session.user_peer_id, owner_peer_id or "unset") return False diff --git a/plugins/memory/honcho/session_peers.py b/plugins/memory/honcho/session_peers.py index c56e108b71..5138904c9f 100644 --- a/plugins/memory/honcho/session_peers.py +++ b/plugins/memory/honcho/session_peers.py @@ -27,6 +27,11 @@ def assistant_peer_id_for(config: Any) -> str: return sanitize_peer_id(getattr(config, "ai_peer", None) or "hermes-assistant") +class HonchoPeerUnresolvedError(RuntimeError): + """No user peer can be named: the transport supplied no runtime identity and honcho.json + declares no peerName. Raised instead of minting a session-derived peer nobody declared.""" + + class SessionPeersMixin: """Resolve user/assistant/observer peer IDs. Reads ``self._config`` and runtime identities only.""" @@ -75,8 +80,10 @@ class SessionPeersMixin: def _resolve_user_peer_id(self, key: str) -> str: """Honcho user peer ID for this manager/session. Order: pinned peerName -> alias of a - runtime identity -> (prefixed) runtime identity -> configured peerName -> session-key fallback.""" - peer_name = self._cfg("peer_name") + runtime identity -> (prefixed) runtime identity -> configured peerName. Raises + HonchoPeerUnresolvedError when none applies: every peer must be one the operator declared + or one the transport supplied, never a name derived from the session key.""" + peer_name = str(self._cfg("peer_name") or "").strip() if peer_name and self._cfg("pin_peer_name", False) is True: return self._sanitize_id(peer_name) @@ -95,8 +102,9 @@ class SessionPeersMixin: if peer_name: return self._sanitize_id(peer_name) - channel, sep, chat_id = key.partition(":") - return self._sanitize_id(f"user-{channel}-{chat_id}" if sep else f"user-default-{key}") + raise HonchoPeerUnresolvedError( + f"Honcho has no user peer for session '{key}': the transport supplied no user identity and " + "honcho.json declares no peerName. Set one with 'hermes honcho peer --user '.") def _resolve_peer_id(self, session: HonchoSession, peer: str | None) -> str: """Resolve a peer alias ('user'/'ai') or explicit peer ID to a concrete, non-empty peer ID.""" diff --git a/tests/honcho_plugin/test_async_memory.py b/tests/honcho_plugin/test_async_memory.py index 4096929f21..ec25a61115 100644 --- a/tests/honcho_plugin/test_async_memory.py +++ b/tests/honcho_plugin/test_async_memory.py @@ -21,6 +21,7 @@ from plugins.memory.honcho.session import ( HonchoSession, HonchoSessionManager, ) +from plugins.memory.honcho.session_peers import HonchoPeerUnresolvedError # --------------------------------------------------------------------------- @@ -519,20 +520,17 @@ class TestMemoryFileMigrationOwnerGate: assert uploaded is False assert honcho_session.upload_file.call_count == 0 - def test_no_declared_owner_single_operator_migrates(self, tmp_path, make_manager): - """No peerName and no runtime identity is the plain CLI install — - the only person who exists is the operator the files describe.""" + def test_no_declared_owner_without_identity_has_no_session_to_migrate(self, tmp_path, make_manager): + """No peerName and no runtime identity: the resolver refuses to name a peer + (#93326), so no session exists for the owner gate and nothing is uploaded.""" mgr = make_manager(write_frequency="turn") - session, honcho_session = _prime_migration_session(mgr, "cli:test", "cli-test") - mgr._peers_cache[session.user_peer_id] = MagicMock() - mgr._peers_cache[session.assistant_peer_id] = MagicMock() - (tmp_path / "MEMORY.md").write_text("memory facts", encoding="utf-8") - uploaded = mgr.migrate_memory_files(session.key, str(tmp_path)) + with pytest.raises(HonchoPeerUnresolvedError): + _prime_migration_session(mgr, "cli:test", "cli-test") - assert uploaded is True - assert honcho_session.upload_file.call_count == 1 + assert mgr.migrate_memory_files("cli:test", str(tmp_path)) is False + assert make_manager.client.session.return_value.upload_file.call_count == 0 def test_aliased_owner_identity_migrates(self, tmp_path, make_manager): """An alias mapping the owner's platform ID onto peerName makes that diff --git a/tests/honcho_plugin/test_auth_recovery.py b/tests/honcho_plugin/test_auth_recovery.py index bc1e68fc2d..5b8077446b 100644 --- a/tests/honcho_plugin/test_auth_recovery.py +++ b/tests/honcho_plugin/test_auth_recovery.py @@ -940,7 +940,7 @@ def _wire_init(tmp_path, monkeypatch, client, *, recall_mode="hybrid", dead_refr monkeypatch.setattr(oauth, "force_refresh_token", lambda p, h, **kw: None) cfg = HonchoClientConfig( host="hermes", api_key="hch-at-old", enabled=True, recall_mode=recall_mode, - timeout=0.5, session_strategy="per-session", + timeout=0.5, session_strategy="per-session", peer_name="operator", ) monkeypatch.setattr( client_mod.HonchoClientConfig, "from_global_config", lambda *a, **k: cfg diff --git a/tests/honcho_plugin/test_pin_peer_name.py b/tests/honcho_plugin/test_pin_peer_name.py index 7530aba74c..1f910e9c5d 100644 --- a/tests/honcho_plugin/test_pin_peer_name.py +++ b/tests/honcho_plugin/test_pin_peer_name.py @@ -18,9 +18,11 @@ import hashlib import json from unittest.mock import MagicMock +import pytest from plugins.memory.honcho.client import HonchoClientConfig from plugins.memory.honcho.session import HonchoSessionManager +from plugins.memory.honcho.session_peers import HonchoPeerUnresolvedError # --------------------------------------------------------------------------- @@ -337,10 +339,11 @@ class TestPeerResolutionOrder: assert session.user_peer_id == "Igor" - def test_everything_missing_falls_back_to_session_key(self): - """Deepest fallback: no runtime identity, no peer_name, no pin. - Must still produce a deterministic peer_id from the session key.""" - # Config with no peer_name and default pin_peer_name=False + @pytest.mark.parametrize("key", ["telegram:123", "rheijo5"]) + def test_everything_missing_refuses_to_mint_a_peer(self, key): + """No runtime identity, no peer_name, no pin: the resolver used to derive + ``user-telegram-123`` / ``user-default-rheijo5`` from the session key, which + put desktop sessions on a phantom peer (#93326). It must refuse instead.""" mgr = HonchoSessionManager( honcho=MagicMock(), config=self._config(peer_name=None, pin_peer_name=False), @@ -348,8 +351,22 @@ class TestPeerResolutionOrder: ) _patch_manager_for_resolution_test(mgr) - session = mgr.get_or_create("telegram:123") - assert session.user_peer_id == "user-telegram-123" + with pytest.raises(HonchoPeerUnresolvedError, match="peerName"): + mgr.get_or_create(key) + assert mgr._get_or_create_peer.call_count == 0 + assert mgr._get_or_create_honcho_session.call_count == 0 + assert key not in mgr._cache + + def test_peer_name_alone_is_the_declared_owner(self): + """No runtime identity: the configured peerName is the peer, unpinned or not.""" + mgr = HonchoSessionManager( + honcho=MagicMock(), + config=self._config(peer_name="Igor", pin_peer_name=False), + runtime_user_peer_name=None, + ) + _patch_manager_for_resolution_test(mgr) + + assert mgr.get_or_create("rheijo5").user_peer_id == "Igor" class TestCrossPlatformMemoryUnification: diff --git a/tests/test_honcho_startup_fail_open.py b/tests/test_honcho_startup_fail_open.py index c4ca1d5d07..f850f8bb8b 100644 --- a/tests/test_honcho_startup_fail_open.py +++ b/tests/test_honcho_startup_fail_open.py @@ -283,6 +283,50 @@ def test_honcho_tools_eager_init_failure_does_not_leave_ready_manager(monkeypatc assert provider._manager is None +def _init_with_unresolved_peer(monkeypatch, cfg) -> tuple[HonchoMemoryProvider, list[int]]: + """Provider whose session init fails because no user peer can be named; returns the attempt log.""" + from plugins.memory.honcho.session_peers import HonchoPeerUnresolvedError + + provider = HonchoMemoryProvider() + monkeypatch.setattr("plugins.memory.honcho.client.HonchoClientConfig.from_global_config", lambda: cfg) + attempts: list[int] = [] + + def no_peer(self, cfg, session_id, **kwargs): + attempts.append(1) + raise HonchoPeerUnresolvedError("Honcho has no user peer for session 'x': honcho.json declares no peerName.") + + monkeypatch.setattr(HonchoMemoryProvider, "_do_session_init", no_peer) + provider.initialize("session-1", platform="cli") + if provider._init_thread: + provider._init_thread.join(timeout=5) + return provider, attempts + + +def test_honcho_unresolved_peer_notices_once_and_stops_retrying(monkeypatch): + """No runtime identity and no peerName: memory stays off for the session, the model hears it + once, and later turns do not re-run init for a config gap that cannot heal (#93326).""" + provider, attempts = _init_with_unresolved_peer(monkeypatch, _configured_hybrid_config()) + + assert provider._manager is None + assert provider._can_start_init() is False + + notice = provider.prefetch("what did we decide about the schema?") + assert "Honcho memory is off" in notice + assert "peerName" in notice + assert provider.prefetch("second question") == "" + provider.sync_turn("hello", "world") + assert attempts == [1] + + +def test_honcho_unresolved_peer_tool_error_names_the_fix(monkeypatch): + provider, _ = _init_with_unresolved_peer(monkeypatch, _configured_tools_config(init_on_session_start=True)) + + result = json.loads(provider.handle_tool_call("honcho_profile", {"peer": "user"})) + + assert "peerName" in result["error"] + assert "could not be initialized" not in result["error"] + + def test_honcho_tools_lazy_hooks_do_not_prestart_background_init(monkeypatch): """tools lazy mode lets the first tool call own session initialization.""" provider = HonchoMemoryProvider()