Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions gateway/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -3356,6 +3356,7 @@ async def _handle_message_with_agent(self, event, source, _quick_key: str):
model=_hyg_model,
max_iterations=4,
quiet_mode=True,
skip_memory=True,
enabled_toolsets=["memory"],
Comment on lines 3356 to 3360

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

skip_memory=True prevents the built-in MemoryStore and memory provider plugins from initializing, but _compress_context() always calls flush_memories() before compression. With skip_memory=True, flush_memories() will always hit MemoryStore is None and the pre-compression memory flush becomes a no-op.

If the intent is to still preserve memory before truncating the transcript, consider initializing/reusing a MemoryStore for this temp agent (without initializing Honcho), or alternatively avoid _compress_context() and call context_compressor.compress() directly when you explicitly want compression to have zero memory side effects.

Copilot uses AI. Check for mistakes.
session_id=session_entry.session_id,
)
Expand Down Expand Up @@ -5758,6 +5759,7 @@ async def _handle_compress_command(self, event: MessageEvent) -> str:
model=model,
max_iterations=4,
quiet_mode=True,
skip_memory=True,

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue as the hygiene temp agent: skip_memory=True means _compress_context()'s pre-compression flush_memories() cannot write anything because self._memory_store was never initialized. If /compress is supposed to preserve durable memory before dropping context, consider wiring a MemoryStore into this temp agent (without initializing Honcho), or bypass _compress_context() when you intentionally want no memory writes/notifications during compression.

Suggested change
skip_memory=True,
skip_memory=False,

Copilot uses AI. Check for mistakes.
enabled_toolsets=["memory"],
session_id=session_entry.session_id,
)
Expand Down Expand Up @@ -7649,6 +7651,7 @@ def _interim_assistant_cb(text: str, *, already_streamed: bool = False) -> None:
session_id=session_id,
platform=platform_key,
user_id=source.user_id,
gateway_session_key=session_key,
session_db=self._session_db,
fallback_model=self._fallback_model,
)
Expand Down
18 changes: 16 additions & 2 deletions plugins/memory/honcho/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,13 @@ def _do_session_init(self, cfg, session_id: str, **kwargs) -> None:

# ----- B3: resolve_session_name -----
session_title = kwargs.get("session_title")
gateway_session_key = kwargs.get("gateway_session_key")
self._session_key = (
cfg.resolve_session_name(session_title=session_title, session_id=session_id)
cfg.resolve_session_name(
session_title=session_title,
session_id=session_id,
gateway_session_key=gateway_session_key,
)
or session_id
or "hermes-default"
)
Expand All @@ -299,12 +304,21 @@ def _do_session_init(self, cfg, session_id: str, **kwargs) -> None:
self._session_initialized = True

# ----- B6: Memory file migration (one-time, for new sessions) -----
# Skip under per-session strategy: every Hermes run creates a fresh
# Honcho session by design, so uploading MEMORY.md/USER.md/SOUL.md to
# each one would flood the backend with short-lived duplicates instead
# of performing a one-time migration.
try:
if not session.messages:
if not session.messages and cfg.session_strategy != "per-session":
from hermes_constants import get_hermes_home
mem_dir = str(get_hermes_home() / "memories")
self._manager.migrate_memory_files(self._session_key, mem_dir)
logger.debug("Honcho memory file migration attempted for new session: %s", self._session_key)
elif cfg.session_strategy == "per-session":
logger.debug(
"Honcho memory file migration skipped: per-session strategy creates a fresh session per run (%s)",
self._session_key,
)
except Exception as e:
logger.debug("Honcho memory file migration skipped: %s", e)

Expand Down
20 changes: 16 additions & 4 deletions plugins/memory/honcho/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -422,16 +422,18 @@ def resolve_session_name(
cwd: str | None = None,
session_title: str | None = None,
session_id: str | None = None,
gateway_session_key: str | None = None,
) -> str | None:
"""Resolve Honcho session name.

Resolution order:
1. Manual directory override from sessions map
2. Hermes session title (from /title command)
3. per-session strategy — Hermes session_id ({timestamp}_{hex})
4. per-repo strategy — git repo root directory name
5. per-directory strategy — directory basename
6. global strategy — workspace name
3. Gateway session key (stable per-chat identifier from gateway platforms)
4. per-session strategy — Hermes session_id ({timestamp}_{hex})
5. per-repo strategy — git repo root directory name
6. per-directory strategy — directory basename
7. global strategy — workspace name
"""
import re

Expand All @@ -451,6 +453,16 @@ def resolve_session_name(
return f"{self.peer_name}-{sanitized}"
return sanitized

# Gateway session key: stable per-chat identifier passed by the gateway
# (e.g. "agent:main:telegram:dm:8439114563"). Sanitize colons to hyphens
# for Honcho session ID compatibility. This takes priority over strategy-
# based resolution because gateway platforms need per-chat isolation that
# cwd-based strategies cannot provide.
if gateway_session_key:
sanitized = re.sub(r'[^a-zA-Z0-9_-]', '-', gateway_session_key).strip('-')
if sanitized:

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The gateway_session_key branch returns the sanitized key without applying session_peer_prefix/peer_name, unlike the session_title/per-session/per-repo/per-directory branches. If sessionPeerPrefix is enabled in config, this effectively gets ignored for gateway-derived sessions and can break the expected isolation/naming scheme.

Consider applying the same peer-prefix rule here as well (when session_peer_prefix and peer_name are set).

Suggested change
if sanitized:
if sanitized:
if self.session_peer_prefix and self.peer_name:
return f"{self.peer_name}-{sanitized}"

Copilot uses AI. Check for mistakes.
return sanitized

# per-session: inherit Hermes session_id (new Honcho session each run)
if self.session_strategy == "per-session" and session_id:
if self.session_peer_prefix and self.peer_name:
Expand Down
6 changes: 6 additions & 0 deletions run_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,7 @@ def __init__(
prefill_messages: List[Dict[str, Any]] = None,
platform: str = None,
user_id: str = None,
gateway_session_key: str = None,
skip_context_files: bool = False,
skip_memory: bool = False,
session_db=None,
Expand Down Expand Up @@ -624,6 +625,7 @@ def __init__(
self.ephemeral_system_prompt = ephemeral_system_prompt
self.platform = platform # "cli", "telegram", "discord", "whatsapp", etc.
self._user_id = user_id # Platform user identifier (gateway sessions)
self._gateway_session_key = gateway_session_key # Stable per-chat key (e.g. agent:main:telegram:dm:123)
# Pluggable print function — CLI replaces this with _cprint so that
# raw ANSI status lines are routed through prompt_toolkit's renderer
# instead of going directly to stdout where patch_stdout's StdoutProxy
Expand Down Expand Up @@ -1163,6 +1165,9 @@ def __init__(
# Thread gateway user identity for per-user memory scoping
if self._user_id:
_init_kwargs["user_id"] = self._user_id
# Thread gateway session key for stable per-chat Honcho session isolation
if self._gateway_session_key:
_init_kwargs["gateway_session_key"] = self._gateway_session_key
# Profile identity for per-profile provider scoping
try:
from hermes_cli.profiles import get_active_profile_name
Expand Down Expand Up @@ -2105,6 +2110,7 @@ def _run_review():
model=self.model,
max_iterations=8,
quiet_mode=True,
skip_memory=True,
platform=self.platform,
provider=self.provider,
)
Expand Down
46 changes: 46 additions & 0 deletions tests/honcho_plugin/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,52 @@ def test_absent_everywhere_defaults_false(self, tmp_path):
assert cfg.init_on_session_start is False


class TestResolveSessionNameGatewayKey:
"""Regression tests for gateway_session_key priority in resolve_session_name.

Ensures gateway platforms get stable per-chat Honcho sessions even when
sessionStrategy=per-session would otherwise create ephemeral sessions.
Regression: plugin refactor 924bc67e dropped gateway key plumbing.
"""

def test_gateway_key_overrides_per_session_strategy(self):
"""gateway_session_key must win over per-session session_id."""
config = HonchoClientConfig(session_strategy="per-session")
result = config.resolve_session_name(
session_id="20260412_171002_69bb38",
gateway_session_key="agent:main:telegram:dm:8439114563",
)
assert result == "agent-main-telegram-dm-8439114563"

def test_session_title_still_wins_over_gateway_key(self):
"""Explicit /title remap takes priority over gateway_session_key."""
config = HonchoClientConfig(session_strategy="per-session")
result = config.resolve_session_name(
session_title="my-custom-title",
session_id="20260412_171002_69bb38",
gateway_session_key="agent:main:telegram:dm:8439114563",
)
assert result == "my-custom-title"

def test_per_session_fallback_without_gateway_key(self):
"""Without gateway_session_key, per-session returns session_id (CLI path)."""
config = HonchoClientConfig(session_strategy="per-session")
result = config.resolve_session_name(
session_id="20260412_171002_69bb38",
gateway_session_key=None,
)
assert result == "20260412_171002_69bb38"

def test_gateway_key_sanitizes_special_chars(self):
"""Colons and other non-alphanumeric chars are replaced with hyphens."""
config = HonchoClientConfig()
result = config.resolve_session_name(
gateway_session_key="agent:main:telegram:dm:8439114563",
)
assert result == "agent-main-telegram-dm-8439114563"
assert ":" not in result


class TestResetHonchoClient:
def test_reset_clears_singleton(self):
import plugins.memory.honcho.client as mod
Expand Down
48 changes: 48 additions & 0 deletions tests/honcho_plugin/test_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,54 @@ def test_user_id_used_when_no_peer_name(self):
assert cfg.peer_name == "8439114563"


class TestPerSessionMigrateGuard:
"""Verify migrate_memory_files is skipped under per-session strategy.

per-session creates a fresh Honcho session every Hermes run. Uploading
MEMORY.md/USER.md/SOUL.md to each short-lived session floods the backend
with duplicate content. The guard was added to prevent orphan sessions
containing only <prior_memory_file> wrappers.
"""

def _make_provider_with_strategy(self, strategy, init_on_session_start=True):
"""Create a HonchoMemoryProvider and track migrate_memory_files calls."""
from plugins.memory.honcho.client import HonchoClientConfig
from unittest.mock import patch, MagicMock

cfg = HonchoClientConfig(
api_key="test-key",
enabled=True,
recall_mode="tools",
init_on_session_start=init_on_session_start,
session_strategy=strategy,
)

provider = HonchoMemoryProvider()

mock_manager = MagicMock()
mock_session = MagicMock()
mock_session.messages = [] # empty = new session → triggers migration path
mock_manager.get_or_create.return_value = mock_session

with patch("plugins.memory.honcho.client.HonchoClientConfig.from_global_config", return_value=cfg), \
patch("plugins.memory.honcho.client.get_honcho_client", return_value=MagicMock()), \
patch("plugins.memory.honcho.session.HonchoSessionManager", return_value=mock_manager), \
patch("hermes_constants.get_hermes_home", return_value=MagicMock()):
provider.initialize(session_id="test-session-001")

return provider, mock_manager

def test_migrate_skipped_for_per_session(self):
"""per-session strategy must NOT call migrate_memory_files."""
_, mock_manager = self._make_provider_with_strategy("per-session")
mock_manager.migrate_memory_files.assert_not_called()

def test_migrate_runs_for_per_directory(self):
"""per-directory strategy with empty session SHOULD call migrate_memory_files."""
_, mock_manager = self._make_provider_with_strategy("per-directory")
mock_manager.migrate_memory_files.assert_called_once()


class TestChunkMessage:
def test_short_message_single_chunk(self):
result = HonchoMemoryProvider._chunk_message("hello world", 100)
Expand Down
Loading