hermes-admin/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 29 Packages Projects Releases Wiki Activity

fix(honcho): gate memory-file migration on the declared owner

Browse Source 
The previous gate compared session.user_peer_id against a fresh
_resolve_user_peer_id() call on the same manager. Both values come from
the same resolver with the same inputs, so a non-owner triggering a new
session in a shared channel passed the check and received the owner's
MEMORY.md/USER.md under their peer.

The owner is now a config fact: _declared_owner_peer_id() returns the
sanitized peerName, and migration runs only when the session's user peer
is that peer. Without a declared peerName, migration runs only when no
runtime gateway identity is present (the single-operator CLI path).
Aliases still work: a platform ID mapped onto peerName resolves to the
owner peer before the comparison.

Tests now derive each session's user peer from the real resolver instead
of hand-picking mismatched ids, so the non-owner test fails against the
old gate.
This commit is contained in:
Erosika
2026-08-12 10:47:20 -04:00
committed by kshitij
parent 27021f5f84
commit 9e77d83354
2 changed files with 162 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

41
plugins/memory/honcho/session.py
View File

@@ -553,6 +553,19 @@ class HonchoSessionManager:
return f"{sanitized_peer_id}-{digest}"
return sanitized_peer_id
def _declared_owner_peer_id(self) -> str | None:
"""Peer ID of the install owner, or None when no owner is declared.
The owner is the identity setup writes as ``peerName``. A runtime
gateway identity is the owner only when an alias maps it onto that
peer — which _resolve_user_peer_id already does, so callers can
compare a session's resolved user peer against this value.
"""
peer_name = getattr(self._config, "peer_name", None) if self._config else None
if peer_name and str(peer_name).strip():
return self._sanitize_id(str(peer_name).strip())
return None
def _resolve_user_peer_id(self, key: str) -> str:
"""Resolve the Honcho user peer ID for this manager/session."""
pin_peer_name = (
@@ -1138,20 +1151,34 @@ class HonchoSessionManager:
return False
# Only migrate the owner-describing memory files (MEMORY.md / USER.md)
# when the session's user peer IS the owner peer. Otherwise a
# when the session's user peer IS the install owner. Otherwise a
# non-owner triggering a new session (e.g. any other human in a shared
# Slack/Discord channel) gets the owner's full profile files uploaded
# under the NON-OWNER's peer, and Honcho's deriver attributes the
# owner's facts to that person. SOUL.md describes the agent, not a
# human, but skipping it here too keeps the migration owner-scoped.
# The owner is resolved through _resolve_user_peer_id (pin/runtime/
# alias aware) — config.peer_name alone is optional and None for
# most single-user setups.
owner_peer_id = self._resolve_user_peer_id(session_key)
if session.user_peer_id != owner_peer_id:
#
# The owner is a CONFIG fact — the declared peerName — never a
# re-resolution of the session's own peer: _resolve_user_peer_id
# answers "who is this session's user", so comparing its output to
# session.user_peer_id compares the triggering user to themselves
# and passes for the non-owner too.
owner_peer_id = self._declared_owner_peer_id()
if owner_peer_id is not None:
session_is_owner = session.user_peer_id == owner_peer_id
else:
# No declared owner. Without a runtime identity this is the
# single-operator path (peer id from config defaults or the
# session key) and the files describe that operator. With a
# runtime identity the session belongs to whoever messaged
# through the gateway — nobody can be proven to be the owner.
session_is_owner = not self._runtime_user_ids()
if not session_is_owner:
logger.info(
"Skipping memory-file migration for non-owner session (user=%s)",
"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

158
tests/honcho_plugin/test_async_memory.py
View File

@@ -54,13 +54,23 @@ def make_manager(monkeypatch):
monkeypatch.setattr(session_module, "get_honcho_client", lambda *a, **k: client)
created = []
def _make(write_frequency="turn") -> HonchoSessionManager:
def _make(
write_frequency="turn",
*,
runtime_user_peer_name=None,
**cfg_kwargs,
) -> HonchoSessionManager:
cfg = HonchoClientConfig(
write_frequency=write_frequency,
api_key="test-key",
enabled=True,
**cfg_kwargs,
)
mgr = HonchoSessionManager(
honcho=client,
config=cfg,
runtime_user_peer_name=runtime_user_peer_name,
)
mgr = HonchoSessionManager(honcho=client, config=cfg)
created.append(mgr)
return mgr
@@ -420,29 +430,35 @@ class TestAsyncWriterRetry:
assert call_count[0] == 2
def _prime_migration_session(mgr, key, honcho_session_id, ai_peer_id="custom-ai"):
"""Cache a session whose user peer is what the REAL resolver returns for
this manager — exactly what get_or_create stores — so the owner gate is
tested against reachable states, not hand-picked peer ids."""
session = _make_session(
key=key,
user_peer_id=mgr._resolve_user_peer_id(key),
assistant_peer_id=ai_peer_id,
honcho_session_id=honcho_session_id,
)
mgr._cache[session.key] = session
honcho_session = MagicMock()
mgr._sessions_cache[session.honcho_session_id] = honcho_session
return session, honcho_session
class TestMemoryFileMigrationTargets:
def test_soul_upload_targets_ai_peer(self, tmp_path, make_manager):
mgr = make_manager(write_frequency="turn")
# Migration is owner-gated: the session's user peer must match what
# _resolve_user_peer_id returns. Make the runtime identity match the
# crafted session so this reads as the owner's own session.
mgr._runtime_user_peer_name = "custom-user"
session = _make_session(
key="cli:test",
user_peer_id="custom-user",
assistant_peer_id="custom-ai",
honcho_session_id="cli-test",
)
mgr._cache[session.key] = session
# peerName declares the owner; no runtime identity, so the session
# resolves to the owner peer and migration proceeds.
mgr = make_manager(write_frequency="turn", peer_name="custom-user")
session, honcho_session = _prime_migration_session(mgr, "cli:test", "cli-test")
assert session.user_peer_id == "custom-user"
user_peer = MagicMock(name="user-peer")
ai_peer = MagicMock(name="ai-peer")
mgr._peers_cache[session.user_peer_id] = user_peer
mgr._peers_cache[session.assistant_peer_id] = ai_peer
honcho_session = MagicMock()
mgr._sessions_cache[session.honcho_session_id] = honcho_session
(tmp_path / "MEMORY.md").write_text("memory facts", encoding="utf-8")
(tmp_path / "USER.md").write_text("user profile", encoding="utf-8")
(tmp_path / "SOUL.md").write_text("ai identity", encoding="utf-8")
@@ -461,26 +477,108 @@ class TestMemoryFileMigrationTargets:
assert peer_by_upload_name["user_profile.md"] is user_peer
assert peer_by_upload_name["agent_soul.md"] is ai_peer
def test_migration_skipped_for_non_owner_session(self, tmp_path, make_manager):
"""A non-owner user peer in the session must not receive the owner's
memory files — see #43752-adjacent shared-channel misattribution."""
mgr = make_manager(write_frequency="turn")
mgr._runtime_user_peer_name = "owner-user"
session = _make_session(
key="discord:shared",
user_peer_id="some-other-human",
assistant_peer_id="custom-ai",
honcho_session_id="shared-chan",
class TestMemoryFileMigrationOwnerGate:
def test_non_owner_gateway_user_is_skipped(self, tmp_path, make_manager):
"""The shared-channel scenario: a declared owner exists, but the
session was triggered by someone else's platform identity. The old
gate (re-resolving the session's own peer) passed here."""
mgr = make_manager(
write_frequency="turn",
peer_name="owner-user",
runtime_user_peer_name="some-other-human",
)
mgr._cache[session.key] = session
mgr._sessions_cache[session.honcho_session_id] = MagicMock()
session, honcho_session = _prime_migration_session(
mgr, "discord:shared", "shared-chan"
)
assert session.user_peer_id == "some-other-human"
(tmp_path / "MEMORY.md").write_text("owner facts", encoding="utf-8")
uploaded = mgr.migrate_memory_files(session.key, str(tmp_path))
assert uploaded is False
assert mgr._sessions_cache[session.honcho_session_id].upload_file.call_count == 0
assert honcho_session.upload_file.call_count == 0
def test_no_declared_owner_with_gateway_identity_is_skipped(
self, tmp_path, make_manager):
"""Without peerName nobody messaging through a gateway can be proven
to be the owner — migration must not run."""
mgr = make_manager(
write_frequency="turn",
runtime_user_peer_name="discord-123",
)
session, honcho_session = _prime_migration_session(
mgr, "discord:shared", "shared-chan"
)
(tmp_path / "MEMORY.md").write_text("owner facts", encoding="utf-8")
uploaded = mgr.migrate_memory_files(session.key, str(tmp_path))
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."""
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))
assert uploaded is True
assert honcho_session.upload_file.call_count == 1
def test_aliased_owner_identity_migrates(self, tmp_path, make_manager):
"""An alias mapping the owner's platform ID onto peerName makes that
gateway identity the owner."""
mgr = make_manager(
write_frequency="turn",
peer_name="owner-user",
user_peer_aliases={"discord-999": "owner-user"},
runtime_user_peer_name="discord-999",
)
session, honcho_session = _prime_migration_session(
mgr, "discord:dm", "discord-dm"
)
assert session.user_peer_id == "owner-user"
mgr._peers_cache[session.user_peer_id] = MagicMock()
mgr._peers_cache[session.assistant_peer_id] = MagicMock()
(tmp_path / "USER.md").write_text("user profile", encoding="utf-8")
uploaded = mgr.migrate_memory_files(session.key, str(tmp_path))
assert uploaded is True
assert honcho_session.upload_file.call_count == 1
def test_pinned_peer_name_migrates(self, tmp_path, make_manager):
"""pinPeerName collapses every identity onto the owner peer by
explicit config, so the files land on the peer they describe."""
mgr = make_manager(
write_frequency="turn",
peer_name="owner-user",
pin_peer_name=True,
runtime_user_peer_name="anyone-at-all",
)
session, honcho_session = _prime_migration_session(
mgr, "discord:shared", "shared-chan"
)
assert session.user_peer_id == "owner-user"
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))
assert uploaded is True
assert honcho_session.upload_file.call_count == 1
# ---------------------------------------------------------------------------