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()