From 939499beed4b7abc3776bb2211c47d43bf1044ed Mon Sep 17 00:00:00 2001 From: Erosika Date: Tue, 26 May 2026 11:04:12 -0400 Subject: [PATCH] chore(honcho): trim PR-history narration from docs and tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove "PR #14984 / #27371 / #1969" references and "the original key / legacy / backwards-compatible / Port #N" narration from the honcho plugin README, tests, and one stale code comment. These artefacts age poorly: they describe how a change happened rather than what the code does today, and they tax readers who weren't around for the original work. Also drop a dangling reference to scratch/memory-plugin-ux-specs.md in __init__.py — the file isn't in the repo or git history. No behaviour change. --- plugins/memory/honcho/README.md | 4 +- plugins/memory/honcho/__init__.py | 6 +-- tests/gateway/test_agent_cache.py | 28 ++++++------- tests/honcho_plugin/test_cli.py | 10 ++--- tests/honcho_plugin/test_pin_peer_name.py | 51 +++++++++-------------- 5 files changed, 40 insertions(+), 59 deletions(-) diff --git a/plugins/memory/honcho/README.md b/plugins/memory/honcho/README.md index 51152581956..dbe3eebc9a5 100644 --- a/plugins/memory/honcho/README.md +++ b/plugins/memory/honcho/README.md @@ -133,8 +133,8 @@ In gateway deployments (Telegram, Discord, Slack, etc.) each user arrives with a | Key | Type | Default | Description | |-----|------|---------|-------------| -| `pinUserPeer` | bool | `false` | When `true`, every gateway runtime user collapses to `peerName`. Single-operator deployments where you want all your platforms (and any other users) to share one peer. Aliased from `pinPeerName` (the original key, still accepted) | -| `pinPeerName` | bool | `false` | Legacy name for `pinUserPeer`. Backwards-compatible; same effect | +| `pinUserPeer` | bool | `false` | When `true`, every gateway runtime user collapses to `peerName`. Single-operator deployments where you want all your platforms (and any other users) to share one peer. Also accepted as `pinPeerName` | +| `pinPeerName` | bool | `false` | Alias for `pinUserPeer`; same effect | | `userPeerAliases` | object | `{}` | Map of runtime IDs to peer IDs (`{"86701400": "eri"}`). Many-to-one is the intended pattern — alias all your runtime IDs to one peer name. One-to-many is not supported; one runtime ID resolves to exactly one peer | | `runtimePeerPrefix` | string | `""` | Prepended to unknown runtime IDs to namespace them (e.g. `"telegram_"` → `telegram_86701400`). Used only when no alias matches. Prevents collisions between platforms whose runtime IDs share the same shape | diff --git a/plugins/memory/honcho/__init__.py b/plugins/memory/honcho/__init__.py index 62696902bde..bbff0d0e628 100644 --- a/plugins/memory/honcho/__init__.py +++ b/plugins/memory/honcho/__init__.py @@ -321,10 +321,8 @@ class HonchoMemoryProvider(MemoryProvider): except Exception as e: logger.debug("Honcho cost-awareness config parse error: %s", e) - # ----- Port #1969: aiPeer sync from SOUL.md — REMOVED ----- - # SOUL.md is persona content, not identity config. aiPeer should - # only come from honcho.json (host block or root) or the default. - # See scratch/memory-plugin-ux-specs.md #10 for rationale. + # aiPeer comes from honcho.json (host block or root) only. + # SOUL.md is persona content, not identity config. # ----- Port #1957: lazy session init for tools-only mode ----- if self._recall_mode == "tools": diff --git a/tests/gateway/test_agent_cache.py b/tests/gateway/test_agent_cache.py index 4ebcda02ce3..6ef601e0dc5 100644 --- a/tests/gateway/test_agent_cache.py +++ b/tests/gateway/test_agent_cache.py @@ -1347,21 +1347,17 @@ class TestCachedAgentInactivityReset: class TestAgentConfigSignatureUserId: - """Regression: shared-thread cache must not reuse an agent across users. + """Shared-thread cache must not reuse an agent across users. - PR #27371 introduces a deterministic per-user-peer resolver in - HonchoSessionManager, but Honcho's resolved runtime user identity is - frozen into the manager at first-message init. When the gateway - session_key intentionally omits the participant ID (the default for - threads via thread_sessions_per_user=False), a cached AIAgent created - by user A is reused for user B's messages, attributing B's writes to - A's resolved Honcho peer. The signature must therefore include - user_id and user_id_alt so per-user agents are built in shared - threads, restoring #27371's per-user-peer contract. + HonchoSessionManager freezes the resolved runtime user identity at + first-message init. When the gateway session_key omits the participant + ID (``thread_sessions_per_user=False``), a cached AIAgent created by + user A would otherwise be reused for user B, attributing B's writes to + A's resolved peer. Including ``user_id`` / ``user_id_alt`` in the + signature forces per-user agent builds in shared threads. - Cost: in a multi-user shared thread, each user triggers a fresh - AIAgent build → cold prompt cache for that user's first turn. The - correctness gain is judged to outweigh the per-user cache warmup. + Tradeoff: cold prompt cache for each user's first turn in a shared + thread, in exchange for correct memory attribution. """ def test_signature_changes_with_user_id(self): @@ -1402,9 +1398,9 @@ class TestAgentConfigSignatureUserId: def test_signature_omits_user_id_when_absent(self): """Default-None user_id must not change signatures vs unset call. - Pre-#27371-fix callers passed no user_id kwarg. Keeping the - default-None signature byte-identical to the previous behavior - avoids invalidating in-flight caches the moment this lands. + Callers that pass no user_id kwarg must produce a signature + byte-identical to ``user_id=None`` so in-flight caches survive + the rollout of this fix. """ from gateway.run import GatewayRunner runtime = {"provider": "anthropic", "api_key": "k", "base_url": "", "api_mode": "chat_completions"} diff --git a/tests/honcho_plugin/test_cli.py b/tests/honcho_plugin/test_cli.py index b97cdcba6c1..24b67679e64 100644 --- a/tests/honcho_plugin/test_cli.py +++ b/tests/honcho_plugin/test_cli.py @@ -157,12 +157,12 @@ class TestCmdStatus: class TestCloneHonchoForProfile: - """Regression tests for clone_honcho_for_profile identity-key carryover. + """Identity-key carryover during profile cloning. - PR #27371 added userPeerAliases, runtimePeerPrefix, and pinPeerName as - host-scoped identity-mapping config. These keys must survive profile - cloning, otherwise a new profile silently fragments memory by resolving - gateway users to raw runtime IDs instead of operator-declared peers. + The host-scoped identity-mapping keys (``userPeerAliases``, + ``runtimePeerPrefix``, ``pinPeerName``) must survive a clone; otherwise + the new profile silently fragments memory by resolving gateway users to + raw runtime IDs instead of operator-declared peers. """ def _setup_clone_env(self, monkeypatch, tmp_path, cfg): diff --git a/tests/honcho_plugin/test_pin_peer_name.py b/tests/honcho_plugin/test_pin_peer_name.py index 858836b29f0..6105734204c 100644 --- a/tests/honcho_plugin/test_pin_peer_name.py +++ b/tests/honcho_plugin/test_pin_peer_name.py @@ -1,22 +1,17 @@ -"""Tests for the ``pinPeerName`` config flag (#14984). +"""Tests for the ``pinPeerName`` / ``pinUserPeer`` config flag. -By default, when Hermes runs under a gateway (Telegram, Discord, Slack, ...) -it passes the platform-native user ID as ``runtime_user_peer_name`` into -``HonchoSessionManager``. That ID wins over any configured ``peer_name`` -so multi-user bots scope memory per user. +Under a gateway (Telegram, Discord, Slack, ...) Hermes passes the +platform-native user ID as ``runtime_user_peer_name`` into +``HonchoSessionManager``. By default that ID wins over any configured +``peer_name`` so multi-user bots scope memory per user. -For a single-user personal deployment where the user connects over multiple -platforms, that default forks memory into one Honcho peer per platform -(Telegram UID, Discord snowflake, Slack user ID, ...). The user asked for -an opt-in knob that pins the user peer to ``peer_name`` from ``honcho.json`` -so the same person's memory stays unified regardless of which platform the -turn arrived on — ``hosts..pinPeerName: true`` (or root-level -``pinPeerName: true``). +For single-user deployments connecting over multiple platforms, +``pinUserPeer: true`` pins the user peer to ``peer_name`` so memory stays +unified across platforms. -These tests exercise both the config parsing (``client.py::from_global_config``) -and the resolution order (``session.py::get_or_create``). We stub the -Honcho API calls so we can assert the chosen ``user_peer_id`` without -touching the network. +Tests cover config parsing (``client.py::from_global_config``) and resolver +order (``session.py::get_or_create``), stubbing Honcho API calls so the +chosen ``user_peer_id`` can be asserted without touching the network. """ import hashlib @@ -406,7 +401,7 @@ class TestPeerResolutionOrder: assert session.honcho_session_id == "telegram-86701400" def test_config_wins_when_pin_is_true(self): - """The #14984 fix: single-user deployments opt into config pinning.""" + """With pin enabled, configured peer_name beats runtime ID.""" mgr = HonchoSessionManager( honcho=MagicMock(), config=self._config( @@ -542,9 +537,8 @@ class TestPeerResolutionOrder: class TestCrossPlatformMemoryUnification: - """The user-visible outcome of the #14984 fix: the same physical user - talking to Hermes via Telegram AND Discord should land on ONE peer - (not two) when pinPeerName is opted in. + """The same physical user talking to Hermes via Telegram AND Discord + lands on ONE peer when ``pinPeerName`` is opted in. """ def _config_pinned(self) -> HonchoClientConfig: @@ -617,15 +611,9 @@ class TestCrossPlatformMemoryUnification: class TestPinUserPeerAlias: - """``pinUserPeer`` is the canonical name; ``pinPeerName`` is the - backwards-compatible alias. - - Both keys land on the same internal ``pin_peer_name`` field. When - both appear, the precedence is: host pinUserPeer → host pinPeerName - → root pinUserPeer → root pinPeerName → default. This matches the - rule for every other host/root override in the plugin and lets a - host block explicitly disable a root-level pin even via the legacy - key. + """``pinUserPeer`` and ``pinPeerName`` both resolve to the same internal + ``pin_peer_name`` field. Precedence when both appear: host pinUserPeer → + host pinPeerName → root pinUserPeer → root pinPeerName → default. """ def test_root_pinUserPeer_true_pins(self, tmp_path): @@ -665,9 +653,8 @@ class TestPinUserPeerAlias: })) config = HonchoClientConfig.from_global_config(config_path=config_file) assert config.pin_peer_name is False, ( - "Host-level pinUserPeer=false must override the legacy " - "root-level pinPeerName=true, otherwise a host can never " - "unpin a globally-pinned profile via the new alias." + "Host-level pinUserPeer=false must override root-level " + "pinPeerName=true so a host can unpin a globally-pinned profile." ) def test_pinPeerName_still_works_unchanged(self, tmp_path):