fix(honcho): isolate clients per profile, resolve baseUrl/apiKey, enforce saveMessages containment - #85452
Merged
kshitijk4poor merged 28 commits intoAug 13, 2026
Conversation
…ing the wrong profile Profile isolation in every multi-profile process (gateway multiplexer, dashboard, cron) is a ContextVar (set_hermes_home_override) that threading.Thread targets cannot see. The plugin's daemon threads — async writer, prefetch, sync, first-turn, init — all funnel through HonchoSessionManager.honcho, which called get_honcho_client() with NO config, re-resolving resolve_config_path()/resolve_active_host() from the ContextVar-blind thread context: every background memory access landed on the DEFAULT profile. Worse, the OAuth paths did the same, so a token refresh on a daemon thread could persist the rotated token into the wrong profile's honcho.json, and a 401 recovery could burn the wrong profile's single-use refresh token. - HonchoClientConfig gains provenance (config_path, hermes_home) captured at resolution time inside the caller's profile scope, with bound_config_path() for consumers - manager.honcho passes the bound config instead of re-resolving - OAuth paths (_apply_fresh_oauth_token, _refresh_cached_oauth, _reauth_required, _force_reauth) use the bound path - the honcho.json timeout memo becomes path-keyed instead of single-slot, so multi-profile processes stop thrashing it and returning profile A's timeout for profile B Groundwork for per-identity client caching (NousResearch#69123, NousResearch#74065); the provenance-field shape follows NousResearch#81401. Co-authored-by: angel12 <angel12@users.noreply.github.com>
…tial fingerprint Replaces the process-wide first-config-wins client singleton with a per-identity slot map. The singleton baked the first profile's workspace_id and bearer into one shared client, so in multi-profile processes (gateway multiplexer, dashboard, cron) every profile's memory landed in whichever workspace initialized first — cross-tenant bleed with no error (NousResearch#69123, NousResearch#74065). cache key: (host, workspace, base_url, environment, provenance paths, effective timeout, credential fingerprint). the fingerprint hashes the OAuth REFRESH token (stable across in-place access-token rotation, changes on re-auth/account switch) or the static api key — so re-running 'hermes honcho setup' to switch accounts produces a new identity instead of silently reusing the old account's client and writing tenant B's data with tenant A's bearer, a hole per-path keys alone cannot close. same-identity slots with a different fingerprint or timeout are EVICTED on replacement, so credential churn can't accumulate pinned clients — the replaced client's pools close when its last holder drops. timeout changes rebuild via the key (the old explicit staleness check is subsumed). failed in-place OAuth rotation resets only the client's own slot. reset_honcho_client() clears everything, preserving test and oauth-flow re-login semantics. per-config-identity caching was first proposed in NousResearch#69142; the provenance-key shape follows NousResearch#81401. this implementation adds the credential fingerprint and eviction they lacked. Co-authored-by: NaMinhyeok <NaMinhyeok@users.noreply.github.com> Co-authored-by: angel12 <angel12@users.noreply.github.com>
Profile isolation is a ContextVar; plain threading.Thread targets start with an empty context, so the plugin's nine daemon threads (session init, prewarm, first-turn base/prefetch, prefetch, sync, memwrite, async writer, context prefetch) resolved ambient state — config path, active host, hermes home, oauth token paths — against the DEFAULT profile whenever they ran under a routed profile's turn. Adds spawn_context_thread(), which copies the caller's context at spawn time so the thread sees the profile scope it was created under, and routes every plugin thread spawn through it. Defense-in-depth under the bound-config work: even ambient resolution on these threads now lands on the right profile. The copy_context approach follows the gateway's own _run_in_executor_with_context pattern; NousResearch#81401 applied it to the init thread, this extends it to all nine spawns. Co-authored-by: angel12 <angel12@users.noreply.github.com>
…anes MemoryManager dispatches provider sync_turn/queue_prefetch work on a single-worker executor and hot prefetch on a plain thread. Neither carried the caller's contextvars, so in multi-profile processes the provider work ran outside the profile's ContextVar-scoped HERMES_HOME override — any ambient resolution inside a provider landed on the default profile. Wrap the submitted callable and the prefetch thread target with contextvars.copy_context().run, mirroring the gateway's _run_in_executor_with_context pattern. Provider-agnostic: benefits every external memory provider, not just Honcho.
Drives the real resolution chain against real honcho.json files under temp HERMES_HOMEs with the same ContextVar override the multiplexer and dashboard use. Pins: - NousResearch#69123's minimal repro: two profile scopes get distinct clients with their own workspaces and bearers - the daemon-thread case: a bound config acquires its profile's client from a thread that cannot see the ContextVar, and spawn_context_thread carries the override where a plain Thread (control test) does not - credential identity: account swap on the same path/host creates a new client and EVICTS the old slot; the OAuth fingerprint survives access-token rotation but changes on re-auth; timeout changes rebuild via the key - provenance capture and its stability outside the profile scope Two-profile repro shape from NousResearch#69142 (NaMinhyeok); scenario set extends the multiplex isolation tests from NousResearch#81401 (angel12). Co-authored-by: NaMinhyeok <NaMinhyeok@users.noreply.github.com> Co-authored-by: angel12 <angel12@users.noreply.github.com>
The dict was written on every build and popped/cleared on eviction and reset, but no read site remained — timeout staleness detection moved into the cache key itself (a timeout change produces a new identity and _slot_for evicts the old slot), which the isolation tests already pin. Flagged in review by @spfcraze.
…fig_path gap - _submit_background and _prefetch_provider: replace unreadable (lambda inner: (lambda: ctx.run(inner)))(fn) with functools.partial(ctx.run, fn) - from_env(): set config_path=resolve_config_path() so bound_config_path() doesn't re-resolve from ContextVar on daemon threads (the exact bug the PR fixes for from_global_config) Review follow-ups for salvaged PR NousResearch#83525.
HonchoClientConfig.from_global_config() only consulted top-level baseUrl / base_url / HONCHO_BASE_URL in ~/.honcho/config.json. The Honcho SDK's native config format — and what Claude Desktop writes — nests the URL at endpoint.baseUrl. Users with that config format had their self-hosted Honcho container silently ignored: every honcho_* call routed to https://api.honcho.dev with a workspace_id that does not exist there, so tools returned empty data with no error anywhere. Resolution order in from_global_config(), highest first: 1. endpoint.baseUrl (SDK-native, what Claude Desktop writes) 2. baseUrl / base_url (root-level, existing behavior) 3. HONCHO_BASE_URL (existing env var) 4. HONCHO_URL (the SDK's own env var, honcho/client.py:234) HONCHO_URL is also read in from_env(). from_global_config() delegates to from_env() whenever the config file is missing or unreadable, so an env fallback wired into only one of the two would silently do nothing for users with no config file. A non-dict endpoint value falls through cleanly rather than raising. Existing users are unaffected — the new sources are consulted only when the existing ones resolve to None. The INFO log for the base_url-unset case now says so explicitly instead of printing only the host. The SDK resolves that case from its own ENVIRONMENTS map (honcho/client.py:36-39), which for environment= production means the public cloud; a self-hosted user whose config was not picked up otherwise sees a healthy-looking startup line. Closes NousResearch#43800.
…back (fixes NousResearch#37436) _resolve_or_create_client() used a plain dict.get(config.host) that fails for dot-form profile host keys (e.g. "hermes.profile_a") even though the _host_block() helper defined nearby handles the legacy dot-form → underscore-form fallback correctly. The result: _host_has_key evaluates to False for every authenticating user, so effective_api_key is set to "local" and every Honcho API call returns 401 Invalid JWT — cascade failure into silent data loss for cross-peer queries and message sync. Fixes by calling the existing _host_block() helper instead of reimplementing the direct lookup. Local variable renamed from _host_block → _host_block_local to avoid shadowing the function. Closes NousResearch#37436
…orm 401 regression Adds the regression test NousResearch#37671 shipped without (dot-form legacy host block must keep its explicit apiKey on local base_urls instead of silently degrading to the 'local' placeholder and 401ing every write), its inverse (no host key -> placeholder), and an invariant test pinning the full resolution order the three adopted fixes compose into: host block > endpoint.baseUrl > flat root > HONCHO_BASE_URL > HONCHO_URL.
Salvage of NousResearch#2757 by @teyrebaz33 — rebased onto current Honcho plugin layout. Stray control characters (e.g. terminal escapes pasted into HONCHO_BASE_URL or config baseUrl) are dropped with a warning so SDK construction cannot crash startup on Invalid non-printable ASCII character errors.
… form) (NousResearch#76414) _all_profile_host_configs() built per-profile host keys inline as f"{HOST}.{profile}" ("hermes.work") while profile_host_key() — used by honcho status/enable/sync and the runtime memory plugin — produces the underscore form ("hermes_work"). The lookup always missed, so 'hermes honcho peers' showed "(not set)" / leaked the raw malformed key into the AI-peer column for every non-default profile. Profile names needing sanitization (dots/spaces) were doubly broken. Verified live: with hosts["hermes_work"] populated, cmd_peers showed 'work ... hermes.work' before the fix and 'work ... hermes' after. Tests: host keys match the writer form, sanitized profile names resolve, peers output shows populated identities with no key leak, and clean fallback for profiles without a block.
… on keyless profile host blocks Two silent-auth-failure paths from NousResearch#36098 (also NousResearch#66125): - the local-URL guard only escaped the 'local' placeholder when the HOST BLOCK had apiKey. A top-level apiKey in honcho.json — explicit user intent, and what 'hermes honcho setup' writes for single-host configs — was dropped on the floor, so AUTH_USE_AUTH self-hosts 401'd on every request. Now any explicit key in honcho.json (host block or top level) is honored; only env-sourced keys are still treated as likely-cloud and skipped for local URLs. - named-profile host blocks do not inherit the default host's apiKey (credential isolation is by design), but the failure was silent: the profile ran unauthenticated and every tool said 'no context'. Affirm isolation and warn loudly at config-resolution time instead, the outcome NousResearch#66125 proposed if inheritance is rejected.
… result' dialectic_query collapsed every backend failure to an empty string, so the explicit honcho_reasoning tool rendered timeouts, server errors, and genuinely-empty answers identically as 'No result from Honcho.' (NousResearch#36098 issue 4). Operators debugging 'search works but reasoning does not' were sent down representation/observation rabbit holes when the real cause was a 30s timeout on a medium-reasoning dialectic call. Add raise_errors to dialectic_query (default false — automatic injection keeps its fail-quiet behavior and cadence backoff) and pass it from the explicit tool call, returning a tool error that names the failure and points at the timeout knob. Auth errors keep their dedicated handler.
The saveMessages knob has been parsed by HonchoClientConfig since its introduction but was never consumed: sync_turn, on_memory_write and on_session_end persisted to Honcho regardless. With saveMessages=false the provider now never writes automatically (raw turns, memory-write conclusion mirroring, session-end flush) while read/tools paths stay fully functional. Guard uses getattr with a True default so legacy/injected configs keep the old behavior. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018giroL5zeMPnPxERxAxXHY
Salvages NousResearch#67559 — original gated sync_turn/on_memory_write/on_session_end but missed shutdown(), whose flush_all() still persisted on exit. hermes-sweeper review (salvageability=high) flagged this as the one gap. Guard sits after the worker-thread joins, not at the top: cleanup is independent of persistence, and a top-of-method return would leak _prefetch_thread/_sync_thread. Adds TestShutdown and clarifies the saveMessages=false README row. Credit @Matroskin86 (original PR author).
The containment commit skipped the whole turn when either side was empty, which would drop a real user message on interrupted or tool-only turns. Keep the guard for fully-empty turns only and skip empty sides individually inside the sync loop.
…er shutdown Provider shutdown() only called manager.flush_all(), which drains the queue but never joins the async-writer thread — manager.shutdown() exists and nothing called it. The writer thread could still be blocked in httpx I/O at interpreter exit (the NousResearch#37632 crash class). Now shutdown() calls manager.shutdown() (flush + join) when persistence is enabled, and a new manager.stop_async_writer() (join only, no flush) when saveMessages is false, so containment and clean teardown compose.
…ager.save() sync_turn called manager._flush_session() directly, which flushes synchronously every turn no matter what writeFrequency says — the "async", "session", and every-N-turns modes were dead configuration on the main turn path. Route through save(), the dispatcher that actually implements those modes. Same bug class reported in NousResearch#19650 (starship-s) and NousResearch#72708 (Diaspar4u); this takes the minimal one-line routing fix without their broader lifecycle refactors. Co-authored-by: starship-s <45587122+starship-s@users.noreply.github.com>
![carnie[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
…ousResearch#801) migrate_memory_files() uploads USER.md/MEMORY.md with peer=user_peer — the session's runtime user. In shared channels, a non-owner's new thread uploads the owner's full profile under the NON-OWNER's peer; Honcho's deriver then attributes the owner's psychometrics/medical/biography to that person. This was the root contamination vector (55/70 contaminated sessions carried the payload). Skip migration unless the session user is the configured owner. SOUL.md unaffected (uploads under assistant peer). Co-authored-by: Minh Nguyen <menhguin@users.noreply.github.com>
The owner gate from NousResearch#82038 compared against config.peer_name directly, which is None for most single-user setups — sanitizing None would raise and the gate never accounted for pinned/runtime/aliased identities. Resolve the owner the same way sessions do, and add the non-owner skip regression test the original PR shipped without. Co-authored-by: menhguin <menhguin@users.noreply.github.com>
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.
on_memory_write spawns a fire-and-forget daemon thread that was never stored on self, so shutdown() couldn't join it — the exact problem the PR fixes for the async writer thread. Store as self._memwrite_thread and include it in the shutdown join loop. Review follow-up for salvaged PR NousResearch#83500.
alt-glitch
added
type/bug
alt-glitch
added
tool/memory
…t from NousResearch#83525 branch) The NousResearch#83525 branch predates two recent main commits. Cherry-picking brought the old versions, reverting: - tui_gateway/methods_profiles.py: server-side ui_meta on profiles.list/configure (NousResearch#85440) - tests/run_agent/test_primary_runtime_restore.py: context-length mock that prevents live network calls during unit tests Restored to origin/main versions.
kshitijk4poor
enabled auto-merge (rebase)
August 13, 2026 17:42
kshitijk4poor
pushed a commit
that referenced
this pull request
Aug 13, 2026
An empty/blank model id reaching get_model_context_length() can't be
meaningfully resolved — and it's worse than a miss: the endpoint
metadata fuzzy matcher ('model in key or key in model') is vacuously
true for "", so it matches an ARBITRARY catalog entry from the live
/v1/models response and returns whatever context length that entry
happens to have, persisting it under a junk '@<base_url>' cache key.
This started failing CI on main when the Nous portal catalog changed:
tests/run_agent/test_primary_runtime_restore.py constructs agents with
model='' against the live portal URL, the arbitrary match now lands on
a 32K entry, and init_agent raises the 64K-floor ValueError
(test_allowed_for_nous_anthropic_messages, red on every PR's slice).
Guard early: a blank model id falls back to DEFAULT_FALLBACK_CONTEXT
immediately, before any cache write or network probe.
Salvaged from #65515 by @whirmill (rebased onto current main; the
guard now sits after the malformed-base_url normalization added since,
and carries an explanatory comment for the fuzzy-match footgun).
Fixes the red slice on #85444, #85452 and every other open PR.
Co-authored-by: whirmill <5079591+whirmill@users.noreply.github.com>
The honcho-ai exclude-newer entry can be added separately if needed. The fix works without it — it only affects uv resolution behavior.
This was referenced Aug 13, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
11 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Consolidates and salvages three Honcho plugin PRs by @erosika (#83500, #83508, #83525) onto latest main, with review follow-up fixes applied on top. All three address distinct bug classes in the Honcho memory plugin that cause silent data corruption in multi-profile/self-hosted setups.
What this PR makes true
The Honcho plugin correctly isolates per-profile clients, resolves self-hosted baseUrl/apiKey configs, and respects saveMessages=false — fixing cross-tenant memory bleed, silent cloud routing, and ignored write containment.
Changes
From PR #83525 — client identity isolation (closes #69123, #74065)
spawn_context_thread()carries contextvars into all 9 plugin daemon threadsfunctools.partial(ctx.run, fn)from_env()now setsconfig_pathsobound_config_path()doesn't re-resolve from ContextVar on daemon threadsFrom PR #83508 — baseUrl/apiKey resolution (closes #43800, #37436, #76414; addresses #36098, #66125)
endpoint.baseUrl(SDK-native, what Claude Desktop writes)HONCHO_URLenv var as fallback_host_block()for dot-form legacy host key fallback in local-auth checkapiKeyfor local URLsraise_errors=Trueindialectic_queryfor explicit tool calls_sanitize_url()drops non-printable chars before client initprofile_host_key()(underscore form)From PR #83500 — saveMessages containment (closes #35209)
saveMessages(sync_turn, on_memory_write, on_session_end, shutdown)sync_turnthroughmanager.save()sowriteFrequencybatching appliesmanager.shutdown()(flush + join writer thread) ormanager.stop_async_writer()(join only, no flush)honcho-memwritethread inshutdown()(was fire-and-forget, the exact problem the PR fixes for the async writer)Attribution
Adopted with original authorship preserved. Contributors credited:
Validation
Closes #83500, #83508, #83525, #69123, #74065, #43800, #37436, #76414, #35209