Skip to content

fix(honcho): use _host_block helper for dot-form legacy host key fallback (fixes #37436) - #37671

Open
Morad37 wants to merge 1 commit into
NousResearch:mainfrom
Morad37:fix/37436-honcho-dot-form-host-fix
Open

fix(honcho): use _host_block helper for dot-form legacy host key fallback (fixes #37436)#37671
Morad37 wants to merge 1 commit into
NousResearch:mainfrom
Morad37:fix/37436-honcho-dot-form-host-fix

Conversation

@Morad37

@Morad37 Morad37 commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a silent auth failure in the Honcho runtime client path. _resolve_or_create_client() reimplemented the host-block lookup as a plain dict.get(config.host) instead of calling the existing _host_block() helper that handles the legacy dot-form-to-underscore-form fallback. When config.host is hermes_profile_a (underscore — the format returned by profile_host_key()) but the JSON config stores it as hermes.profile_a (dot — the old format the helper is designed to bridge), the direct lookup misses and effective_api_key is set to "local" — every Honcho API call returns 401, silently dropping cross-peer queries and message sync.

Changes

  • plugins/memory/honcho/client.py: 2-line change — replace the bare (_raw.get("hosts") or {}).get(config.host, {}) with _host_block(_raw, config.host). Renamed the local from _host_block to _host_block_local to avoid shadowing the function.

How to test

  1. ~/.hermes/hermes-agent/venv/bin/python -m pytest tests/honcho_plugin/ -q --timeout=120 — 316/316 pass, 17 skipped (unchanged).

Notes

Closes #37436

…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
@alt-glitch alt-glitch added tool/memory Memory tool and memory providers comp/plugins Plugin system and bundled plugins type/bug Something isn't working P3 Low — cosmetic, nice to have labels Jun 2, 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 inconsistent local-auth lookup. The production change matches the existing resolver behavior: current main's _host_block() supports the dot-form fallback at plugins/memory/honcho/client.py:44-51, while the local branch bypasses it at plugins/memory/honcho/client.py:884-887.

Problems

  • This PR has no regression test. The existing dot-form test at tests/honcho_plugin/test_client.py:530-544 verifies config loading only; it does not assert that a localhost get_honcho_client() call passes the legacy host's configured key to the SDK.
  • The equivalent OAuth refresh reads still use direct host lookups at plugins/memory/honcho/oauth.py:279 and :292. get_honcho_client() invokes that refresh at plugins/memory/honcho/client.py:754 and :771, so legacy dot-form OAuth grants remain unable to refresh.

Suggested changes

  • Add the localhost legacy-host API-key construction regression test.
  • Apply the same legacy-key resolution to OAuth reads and preserve the resolved legacy block when persisting a rotated credential.

Automated hermes-sweeper review.

_raw = config.raw or {}
_host_block = (_raw.get("hosts") or {}).get(config.host, {})
_host_has_key = bool(_host_block.get("apiKey"))
_host_block_local = _host_block(_raw, config.host) # uses dot-form legacy fallback (#37436)

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.

Please add a regression test that constructs a localhost client from hosts.hermes.profile_a with active host hermes_profile_a and asserts the SDK receives the configured API key. The current dot-form test covers only from_global_config, not this loopback auth branch.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data 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 area/memory Memory subsystem: store, providers, sync, background reviews labels Jul 13, 2026
kshitijk4poor pushed a commit that referenced this pull request Aug 13, 2026
…orm 401 regression

Adds the regression test #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.
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 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-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data tool/memory Memory tool and memory providers type/bug Something isn't working

Projects

None yet

3 participants