feat(honcho): add sessionAiPeerPrefix to isolate sessions per AI peer - #39130
feat(honcho): add sessionAiPeerPrefix to isolate sessions per AI peer#39130itnkr wants to merge 2 commits into
Conversation
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved
Clean, backward-compatible config addition: sessionAiPeerPrefix mirrors the existing sessionPeerPrefix behavior but for AI peers. Resolver refactor is sound (delegates to _resolve_session_name_base then post-processes) and tests cover the gateway-key collision case thoroughly.
Already docs-updated in a separate PR (39128), so the user-facing narrative is consistent.
Reviewed by Hermes Agent
|
Thanks for splitting this as a narrow opt-in fix. I re-checked the current Honcho/profile landscape while dogfooding multiple Hermes profiles, and this is the cleanest patch for the session collision layer. Observed shape on current main:
I also like that the PR applies the prefix after the base resolver and re-runs the length cap; that avoids making the gateway path a one-off special case. For reviewer context: this is complementary to #30077 (runtime user-peer mapping) and not a replacement for profile/workspace isolation. It is the small hardening I would enable before turning on any automatic/context/hybrid Honcho recall in a multi-profile setup. |
6d23ade to
35390ad
Compare
|
Rebased onto current Some real-world signal worth adding: this patch has been running in production across my full Hermes deployment for ~3 weeks — five Dockerized spheres plus a native macOS install, all now on 0.17.0. One sphere runs six AI peers sharing a single Would appreciate a maintainer look whenever there is a window 🙏 |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the narrowly scoped, default-off session-isolation fix. The current gateway path does have the stated collision: plugins/memory/honcho/client.py:698-701 returns a sanitized gateway_session_key without incorporating ai_peer.
Problems
- The new setting needs gateway cache invalidation.
gateway/run.py:15756-15762enumerates the Honcho settings included in the cached-agent signature, and the PR does not addsessionAiPeerPrefix. That signature controls reuse atgateway/run.py:18158-18163; meanwhile the provider resolves and retains its session key atplugins/memory/honcho/__init__.py:433-445. A live config flip would therefore leave an existing gateway session on its old Honcho session until unrelated eviction or restart. The analogous contract is explicitly covered for other Honcho identity changes intests/honcho_plugin/test_pin_peer_name.py:672-817.
Suggested changes
- Add
honcho.session_ai_peer_prefixto the Honcho cache-busting extraction ingateway/run.py, and add a config-flip regression test alongside the existing Honcho cache-signature tests.
Automated hermes-sweeper review.
| # sessions disjoint when several AI peers share one workspace + peerName + | ||
| # gateway chat key; without it the gateway_session_key branch in | ||
| # resolve_session_name() yields the same name for every AI peer. | ||
| session_ai_peer_prefix: bool = False |
There was a problem hiding this comment.
Please also include this setting in the gateway's Honcho cache-busting signature (gateway/run.py:15756-15794) and test a honcho.json false→true flip. A cached gateway AIAgent retains the Honcho manager/session key resolved at initialization, so without a signature change this opt-in setting will not affect an already-active gateway session until unrelated eviction or restart.
|
Addressed the cache-invalidation feedback: added While in there I also closed the same latent gap for the pre-existing user-side |
70d44d2 to
ae949b5
Compare
|
Rebased onto current The earlier review feedback is addressed: Ready for another look whenever you have a moment. 🙏 |
ae949b5 to
bfae3ad
Compare
|
Rebased onto current Conflict resolution. Only Verified the clean auto-merges, since two of them were load-bearing:
Two small additions while rebasing:
|
Address review feedback on NousResearch#39130: the new sessionAiPeerPrefix setting affects the resolved Honcho session key, which HonchoMemoryProvider freezes at construction (self._session_key). Because it wasn't part of the gateway's cached-agent signature, a live config flip left an existing gateway session bound to its old, AI-peer-agnostic Honcho session until an unrelated eviction or restart. Add honcho.session_ai_peer_prefix to _HONCHO_CACHE_BUSTING_KEYS and the _extract_honcho_cache_busting_config values so a flip rebuilds the cached agent on the next turn, mirroring the existing aiPeer / pin_peer_name / runtime_peer_prefix contracts. Also close the symmetric gap for the pre-existing user-side sessionPeerPrefix: it feeds the same resolve_session_name output (per-session/title/per-repo/ per-directory strategies) into the same frozen _session_key, so it had the identical live-flip staleness bug and was likewise absent from the cache signature. Fixing both keeps the two prefixes consistent. Add one config-flip regression test covering both keys, alongside the existing Honcho cache-signature test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The gateway_session_key branch of resolve_session_name() returns an
AI-peer-agnostic name, so multiple AI peers sharing one workspace +
peerName + gateway chat key collide on a single Honcho session.
Add sessionAiPeerPrefix (symmetric counterpart to sessionPeerPrefix):
when set, the resolved session name is prefixed with {ai_peer}- on every
resolution path. The prefixed name is re-run through the session-id
length cap so the prefix can never exceed Honcho's limit.
- config field + host/root parsing in client.py
- public resolve_session_name() wraps a new _resolve_session_name_base()
- tests covering parsing, the gateway-key case, cross-peer disjointness,
the length cap, and a disabled-by-default regression guard
- README: config table + resolution notes
Address review feedback on NousResearch#39130: the new sessionAiPeerPrefix setting affects the resolved Honcho session key, which HonchoMemoryProvider freezes at construction (self._session_key). Because it wasn't part of the gateway's cached-agent signature, a live config flip left an existing gateway session bound to its old, AI-peer-agnostic Honcho session until an unrelated eviction or restart. Add honcho.session_ai_peer_prefix to _HONCHO_CACHE_BUSTING_KEYS and the _extract_honcho_cache_busting_config values so a flip rebuilds the cached agent on the next turn, mirroring the existing aiPeer / pin_peer_name / runtime_peer_prefix contracts. Also close the symmetric gap for the pre-existing user-side sessionPeerPrefix: it feeds the same resolve_session_name output (per-session/title/per-repo/ per-directory strategies) into the same frozen _session_key, so it had the identical live-flip staleness bug and was likewise absent from the cache signature. Fixing both keeps the two prefixes consistent. Add one config-flip regression test covering both keys, alongside the existing Honcho cache-signature test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
bfae3ad to
5951fd3
Compare
|
Rebased onto current Rebase notes. ~4,035 commits of drift since the 07-30 rebase, and nothing in the overlay's context moved: all three touched files ( Tests. 36/36 across |
What does this PR do?
Adds an opt-in
sessionAiPeerPrefixconfig flag for the Honcho memory plugin. When set, the resolved Honcho session name is prefixed with{aiPeer}-so that multiple AI peers sharing oneworkspace+peerName+ gateway chat key stay on disjoint sessions instead of colliding on a single one.It is the symmetric counterpart to the existing
sessionPeerPrefix(which prefixes the user peer). The motivating case is thegateway_session_keybranch ofresolve_session_name(), which is AI-peer-agnostic — so without this flag there is no way to isolate two agents that share a workspace and peer name.Related Issue
Fixes #39129
Type of Change
Default is
false; when unset, session-name resolution is byte-for-byte unchanged.Changes Made
plugins/memory/honcho/client.pysession_ai_peer_prefixconfig fieldsessionAiPeerPrefix(mirrorssessionPeerPrefix)resolve_session_name()is now a thin wrapper that delegates to a new_resolve_session_name_base()(the existing resolution ladder, unchanged) and then applies the{aiPeer}-prefix on every path. The prefixed name is re-run through_enforce_session_id_limit()so the prefix can never push a name past Honcho's session-id cap.tests/honcho_plugin/test_session_ai_peer_prefix.py(new)plugins/memory/honcho/README.md— config table + resolution notesHow to Test
Tests cover: config parsing (default/root/host/host-overrides-root), the gateway-key collision case, cross-peer disjointness (same chat key + different
aiPeer→ different sessions), the session-id length cap, and a disabled-by-default regression guard. Existing honcho suites (test_honcho_client_config.py,test_pin_peer_name.py) still pass — 61 green locally.Checklist
false)