From 326c9daa695b6459285781b1f02e84914db581b9 Mon Sep 17 00:00:00 2001 From: "Brian D. Evans" <252620095+briandevans@users.noreply.github.com> Date: Fri, 24 Apr 2026 06:52:46 -0700 Subject: [PATCH] fix(honcho): require strict True for pin_peer_name to survive MagicMock configs (#15162) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI caught that ``test_session_manager_prefers_runtime_user_id_over_config_peer_name`` in ``tests/agent/test_memory_user_id.py`` failed after this branch: that test passes a ``MagicMock`` for ``config``, where ``mock.pin_peer_name`` silently returns another ``MagicMock`` — truthy by default. My ``getattr(..., "pin_peer_name", False)`` fallback was supposed to guard against callers that haven't added the new attr, but MagicMock *does* have the attr — it just returns a live mock for it. Tightened the gate to ``getattr(..., False) is True``. Real configs built via ``HonchoClientConfig.from_global_config`` always yield a proper boolean, so strict equality matches the pinned case and rejects both the unset-attr fallback and MagicMock stand-ins. Added a comment explaining why ``is True`` is intentional, not paranoid. Also tightened the ``peer_name`` existence check to ``getattr(..., None)`` so a MagicMock with ``peer_name`` left at its default (also truthy) doesn't spuriously enable pinning either. Verified against both the new ``test_pin_peer_name.py`` suite (13/13 pass) and the previously-failing ``TestHonchoUserIdScoping`` (3/3 pass). Zero behaviour change for real ``HonchoClientConfig`` values. Co-Authored-By: Claude Opus 4.7 (1M context) --- plugins/memory/honcho/session.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/plugins/memory/honcho/session.py b/plugins/memory/honcho/session.py index 9d4fa41eb..55b9e0d18 100644 --- a/plugins/memory/honcho/session.py +++ b/plugins/memory/honcho/session.py @@ -284,10 +284,16 @@ class HonchoSessionManager: # identity and we should keep it unified across platforms — see # #14984. Opt into that with ``hosts..pinPeerName: true`` in # ``honcho.json`` (or root-level ``pinPeerName: true``). - pin_peer_name = bool( - self._config - and self._config.peer_name - and getattr(self._config, "pin_peer_name", False) + # `is True` (not `bool(...)`) is deliberate: several multi-user tests + # pass a ``MagicMock`` for ``config`` where ``mock.pin_peer_name`` + # silently returns another MagicMock — truthy by default. Requiring + # strict ``True`` keeps pinning as opt-in even for callers that + # haven't updated their mocks yet; real configs built via + # ``from_global_config`` always produce a proper boolean. + pin_peer_name = ( + self._config is not None + and bool(getattr(self._config, "peer_name", None)) + and getattr(self._config, "pin_peer_name", False) is True ) if self._runtime_user_peer_name and not pin_peer_name: user_peer_id = self._sanitize_id(self._runtime_user_peer_name)