mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-28 06:51:16 +08:00
fix(honcho): require strict True for pin_peer_name to survive MagicMock configs (#15162)
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) <noreply@anthropic.com>
This commit is contained in:
@@ -284,10 +284,16 @@ class HonchoSessionManager:
|
|||||||
# identity and we should keep it unified across platforms — see
|
# identity and we should keep it unified across platforms — see
|
||||||
# #14984. Opt into that with ``hosts.<host>.pinPeerName: true`` in
|
# #14984. Opt into that with ``hosts.<host>.pinPeerName: true`` in
|
||||||
# ``honcho.json`` (or root-level ``pinPeerName: true``).
|
# ``honcho.json`` (or root-level ``pinPeerName: true``).
|
||||||
pin_peer_name = bool(
|
# `is True` (not `bool(...)`) is deliberate: several multi-user tests
|
||||||
self._config
|
# pass a ``MagicMock`` for ``config`` where ``mock.pin_peer_name``
|
||||||
and self._config.peer_name
|
# silently returns another MagicMock — truthy by default. Requiring
|
||||||
and getattr(self._config, "pin_peer_name", False)
|
# 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:
|
if self._runtime_user_peer_name and not pin_peer_name:
|
||||||
user_peer_id = self._sanitize_id(self._runtime_user_peer_name)
|
user_peer_id = self._sanitize_id(self._runtime_user_peer_name)
|
||||||
|
|||||||
Reference in New Issue
Block a user