fix(honcho): align peer-card read and write paths
honcho_profile(peer="user") returned an empty card even when Honcho
held a populated peer card for the user. Two independent bugs combined
to produce the symptom:
1. Read path: get_peer_card() called _fetch_peer_card(observer, target=user),
which hits GET /peers/{observer}/card?target={user} — the observer's local
card of the user. On self-hosted Honcho v3 this slot is empty unless writes
also use it. The peer card lives on the user peer itself
(GET /peers/{user}/card). Add a fallback: when the observer-target slot is
empty and a target exists, retry against the target peer's own card.
2. Write path: set_peer_card() resolved only the target peer and called
user_peer.set_card(card). The read path uses the assistant peer as
observer, so writes and reads addressed different Honcho card scopes.
Align set_peer_card() with _resolve_observer_target() so writes go to
assistant_peer.set_card(card, target=user_peer_id), matching the read.
Both paths now use the same observer/target resolution, and the read
path additionally falls back to the target's own card for compatibility
with deployments where cards were written directly to the peer.
Closes: related to #13375, #17124, #20729
This commit is contained in:
@ -1087,7 +1087,17 @@ class HonchoSessionManager:
|
||||
|
||||
try:
|
||||
observer_peer_id, target_peer_id = self._resolve_observer_target(session, peer)
|
||||
return self._fetch_peer_card(observer_peer_id, target=target_peer_id)
|
||||
card = self._fetch_peer_card(observer_peer_id, target=target_peer_id)
|
||||
if card:
|
||||
return card
|
||||
# Honcho self-hosted v3 stores the peer card on the peer itself
|
||||
# (GET /peers/{id}/card). The observer-target slot used above is
|
||||
# only populated when writes also go through that path. Fall back
|
||||
# to the target peer's own card so honcho_profile works regardless
|
||||
# of which write path populated it.
|
||||
if target_peer_id:
|
||||
return self._fetch_peer_card(target_peer_id)
|
||||
return []
|
||||
except Exception as e:
|
||||
logger.debug("Failed to fetch peer card from Honcho: %s", e)
|
||||
return []
|
||||
@ -1234,13 +1244,22 @@ class HonchoSessionManager:
|
||||
if not session:
|
||||
return None
|
||||
try:
|
||||
peer_id = self._resolve_peer_id(session, peer)
|
||||
if peer_id is None:
|
||||
observer_peer_id, target_peer_id = self._resolve_observer_target(session, peer)
|
||||
if observer_peer_id is None:
|
||||
logger.warning("Could not resolve peer '%s' for set_peer_card in session '%s'", peer, session_key)
|
||||
return None
|
||||
peer_obj = self._get_or_create_peer(peer_id)
|
||||
result = peer_obj.set_card(card)
|
||||
logger.info("Updated peer card for %s (%d facts)", peer_id, len(card))
|
||||
peer_obj = self._get_or_create_peer(observer_peer_id)
|
||||
result = (
|
||||
peer_obj.set_card(card, target=target_peer_id)
|
||||
if target_peer_id is not None
|
||||
else peer_obj.set_card(card)
|
||||
)
|
||||
logger.info(
|
||||
"Updated peer card observer=%s target=%s (%d facts)",
|
||||
observer_peer_id,
|
||||
target_peer_id or observer_peer_id,
|
||||
len(card),
|
||||
)
|
||||
return result
|
||||
except Exception as e:
|
||||
logger.error("Failed to set peer card: %s", e)
|
||||
|
||||
@ -212,6 +212,39 @@ class TestPeerLookupHelpers:
|
||||
assert mgr.get_peer_card(session.key) == ["Name: Robert"]
|
||||
assistant_peer.get_card.assert_called_once_with(target=session.user_peer_id)
|
||||
|
||||
def test_get_peer_card_falls_back_to_target_peer_own_card(self):
|
||||
# When the observer-target card slot is empty (returns None/[]), fall
|
||||
# back to the target peer's own card. Self-hosted Honcho v3 stores the
|
||||
# peer card on the peer itself; the observer-target slot is only
|
||||
# populated when writes also go through that path.
|
||||
mgr, session = self._make_cached_manager()
|
||||
assistant_peer = MagicMock()
|
||||
assistant_peer.get_card.return_value = None # observer-target slot empty
|
||||
user_peer = MagicMock()
|
||||
user_peer.get_card.return_value = ["Prefers: dark mode"]
|
||||
|
||||
def _peer(peer_id: str) -> MagicMock:
|
||||
return assistant_peer if peer_id == session.assistant_peer_id else user_peer
|
||||
|
||||
mgr._get_or_create_peer = MagicMock(side_effect=_peer)
|
||||
|
||||
assert mgr.get_peer_card(session.key) == ["Prefers: dark mode"]
|
||||
assistant_peer.get_card.assert_called_once_with(target=session.user_peer_id)
|
||||
user_peer.get_card.assert_called_once_with()
|
||||
|
||||
def test_set_peer_card_uses_observer_target_in_ai_observe_others_mode(self):
|
||||
# Writes must go to the same observer-target slot that reads check,
|
||||
# so that a subsequent honcho_profile read returns what was written.
|
||||
mgr, session = self._make_cached_manager()
|
||||
assistant_peer = MagicMock()
|
||||
assistant_peer.set_card.return_value = ["Role: user"]
|
||||
mgr._get_or_create_peer = MagicMock(return_value=assistant_peer)
|
||||
|
||||
result = mgr.set_peer_card(session.key, ["Role: user"])
|
||||
|
||||
assert result == ["Role: user"]
|
||||
assistant_peer.set_card.assert_called_once_with(["Role: user"], target=session.user_peer_id)
|
||||
|
||||
def test_search_context_uses_assistant_perspective_with_target(self):
|
||||
mgr, session = self._make_cached_manager()
|
||||
assistant_peer = MagicMock()
|
||||
|
||||
Reference in New Issue
Block a user