Skip to content

fix(honcho): canonicalize gateway session keys - #50134

Open
rzyns wants to merge 3 commits into
NousResearch:mainfrom
rzyns:fix/honcho-gateway-session-keys
Open

fix(honcho): canonicalize gateway session keys#50134
rzyns wants to merge 3 commits into
NousResearch:mainfrom
rzyns:fix/honcho-gateway-session-keys

Conversation

@rzyns

@rzyns rzyns commented Jun 21, 2026

Copy link
Copy Markdown

What does this PR do?

Fixes two Honcho session-identity issues that can fragment gateway/WebUI memory writeback:

  1. HonchoMemoryProvider now refreshes its cached session key when the active Hermes session changes, so later sync/async writes do not keep using the previous session's Honcho key.
  2. HonchoClientConfig.resolve_session_name() now treats gateway_session_key as the canonical storage key for gateway/WebUI turns when present, ahead of display session_title. Titles remain a legacy/non-gateway fallback and are exposed as readback candidates.

This keeps explicit sessions overrides highest priority and preserves legacy candidate visibility through a read-only resolver/CLI path instead of migrating historical Honcho sessions.

Related Issue

No single issue filed for this exact two-part bug.

Related open Honcho PRs found during duplicate search:

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • plugins/memory/honcho/__init__.py
    • Adds on_session_switch(...) lifecycle handling.
    • Clears session-scoped prefetch/cache state on switch.
    • Captures the active session key before background sync writes.
  • plugins/memory/honcho/client.py
    • Adds resolver candidates with source labels.
    • Changes precedence to explicit session override → gateway_session_keysession_title → strategy fallback.
    • Keeps gateway keys sanitized/prefixed/length-limited.
  • plugins/memory/honcho/cli.py
    • Adds read-only hermes honcho resolve-session diagnostics for primary and legacy candidate keys.
  • plugins/memory/honcho/README.md
    • Updates the documented session-name resolution order and diagnostics.
  • tests/honcho_plugin/*
    • Adds regression coverage for session-switch writeback, resolver precedence, candidate readback, CLI diagnostics, prefixing, and length limiting.

How to Test

Targeted Honcho validation run against this PR branch:

  1. PYTHONPATH=. /home/openclaw/.hermes/hermes-agent/venv/bin/python -m pytest tests/honcho_plugin/test_async_memory.py tests/honcho_plugin/test_cli.py tests/honcho_plugin/test_client.py -q
    • PASS: 180 passed, 3 warnings
  2. PYTHONPATH=. /home/openclaw/.hermes/hermes-agent/venv/bin/python -m pytest tests/honcho_plugin -q
    • PASS: 348 passed, 3 warnings
  3. PYTHONPATH=. /home/openclaw/.hermes/hermes-agent/venv/bin/python -m pytest tests/test_honcho*.py tests/honcho_plugin -q
    • PASS: 371 passed, 3 warnings
  4. git diff --check upstream/main..HEAD
    • PASS

Warnings were dependency deprecations from discord, lark_oapi, and websockets.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: WSL2 / Linux

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

For New Skills

N/A

Screenshots / Logs

N/A — CLI/provider behavior covered by tests.

AI-Assisted Disclosure

AI assistance was used to inspect the existing Honcho code paths, prepare the patch, resolve the upstream rebase conflict, and draft this PR body. The final diff was reviewed locally, kept scoped to Honcho session identity/writeback behavior, and validated with the targeted test commands listed above.

Refresh the Honcho provider's cached session key on session switches and prefer stable gateway_session_key values over display titles when resolving gateway/WebUI Honcho storage keys. Add read-only diagnostics and regression coverage for legacy candidate lookup.
@alt-glitch alt-glitch added type/bug Something isn't working comp/plugins Plugin system and bundled plugins tool/memory Memory tool and memory providers P3 Low — cosmetic, nice to have labels Jun 21, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for isolating the cached-session-key writeback problem. The current-main premise is valid, but this branch needs rework before salvage.

Problems

  • plugins/memory/honcho/client.py:773 makes a manual map win before per-session identity. Current main deliberately makes session_id authoritative in that mode (plugins/memory/honcho/client.py:703-722; tests/honcho_plugin/test_async_memory.py:158-170). Retain that contract; gateway-key precedence is already implemented on main (plugins/memory/honcho/client.py:696-701).
  • plugins/memory/honcho/__init__.py:1145 only carries switch inputs from _lazy_init_kwargs, but the initializer clears it at PR line 420. The switch fan-out does not pass gateway_session_key (agent/memory_manager.py:871-875), so a ready gateway session will resolve to the new transcript id instead of its stable gateway key. This conflicts with the API-server contract that gateway keys persist while transcript ids rotate (gateway/platforms/api_server.py:1282-1287).

Suggested changes

  • Preserve the current resolver semantics and focus the salvage on Honcho's missing on_session_switch implementation.
  • Store initial resolution inputs independently of lazy-init state, merge them on switch, and test the actual MemoryManager switch path after successful initialization.

Automated hermes-sweeper review.

Comment thread plugins/memory/honcho/client.py Outdated

# Explicit/manual keys and gateway keys are source-level overrides that
# beat presentation metadata and strategy-based fallback resolution.
if candidate_map.get("manual_override"):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Current main makes per-session session_id authoritative over both a manual map and a title (tests/honcho_plugin/test_async_memory.py:158-170). Returning manual_override first reverses that current contract; retain the existing per-session precedence while preserving gateway-key priority.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed in 13504afbb: I removed the resolver changes from this PR, so client.py now matches current main and retains the existing precedence (gateway_session_key first, then authoritative per-session session_id, then manual map/title for non-per-session strategies). The remaining change is limited to the provider lifecycle/session-switch fix and its regression tests.

Comment thread plugins/memory/honcho/__init__.py Outdated
# finishes and discards stale manager/key state.
self._session_generation += 1

switch_kwargs = dict(self._lazy_init_kwargs or {})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

_lazy_init_kwargs is cleared after successful background initialization (line 420), and MemoryManager.on_session_switch() does not forward gateway_session_key. A ready gateway session therefore resolves from the new transcript id here. Persist the initial resolution inputs separately and add a test through the manager switch path.

# Conflicts:
#	plugins/memory/honcho/client.py
#	tests/honcho_plugin/test_async_memory.py
#	tests/honcho_plugin/test_client.py
@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
@rzyns

rzyns commented Jul 14, 2026

Copy link
Copy Markdown
Author

Thanks for the detailed review — I reworked the PR in 13504afbb.

The previous resolver/CLI changes have been removed, so the resolver keeps current main semantics: gateway_session_key wins first, while Hermes session_id remains authoritative in per-session mode.

The remaining change is focused on the missing Honcho lifecycle behavior:

  • added HonchoMemoryProvider.on_session_switch();
  • preserved the initial gateway/session resolution inputs independently of _lazy_init_kwargs, so they remain available after successful initialization clears lazy state;
  • merged those stored inputs into subsequent session switches;
  • added regression coverage through the actual MemoryManager.on_session_switch() path after successful initialization;
  • verified that normal per-session providers rebind to the new transcript ID, while gateway-backed providers continue writing to the stable gateway-derived Honcho key.

Validation: the full Honcho plugin suite passes (373 passed), Ruff passes, and the PR diff is now limited to plugins/memory/honcho/__init__.py plus the focused regression tests.

Could you please take another look when you have a chance?

@teknium1 teknium1 added area/sessions Session lifecycle, resume, persistence, history area/memory Memory subsystem: store, providers, sync, background reviews labels Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/memory Memory subsystem: store, providers, sync, background reviews area/sessions Session lifecycle, resume, persistence, history comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state tool/memory Memory tool and memory providers type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants