Skip to content

fix(honcho): respect dot-form host api keys - #37456

Closed
westkite1201 wants to merge 5 commits into
NousResearch:mainfrom
westkite1201:fix/honcho-dot-host-local-api-key
Closed

westkite1201 wants to merge 5 commits into
NousResearch:mainfrom
westkite1201:fix/honcho-dot-host-local-api-key

Conversation

@westkite1201

@westkite1201 westkite1201 commented Jun 2, 2026 •

Copy link
Copy Markdown

What does this PR do?

Fixes local Honcho client initialization for legacy dot-form profile host blocks.

When the active host is the safe profile name hermes_profile_a, HonchoClientConfig.from_global_config() already resolves legacy config blocks such as hosts.hermes.profile_a and loads the configured apiKey. However, the local-base-url branch in get_honcho_client() re-checked host config with a direct hosts[config.host] lookup, so it missed the same legacy dot-form block and replaced the configured key with the SDK placeholder local.

This reuses the existing _host_block() resolver in that final local-auth check, keeping behavior consistent with config loading while preserving the no-auth local fallback when no host-level apiKey exists.

Related Issue

Fixes #37436

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/client.py
    • Use _host_block(config.raw, config.host) when deciding whether a local Honcho base URL should pass the configured host apiKey or the local placeholder.
  • tests/honcho_plugin/test_client.py
    • Add a regression test for hermes_profile_a resolving a legacy hosts.hermes.profile_a.apiKey block and passing that key to the Honcho SDK for a localhost base URL.

How to Test

  1. Reproduce the pre-fix failure with the new regression test: before the production change, the captured SDK api_key is local instead of jwt-for-local-honcho.
  2. Run the focused regression test:
    /home/seo/.hermes/hermes-agent/venv/bin/python -m pytest tests/honcho_plugin/test_client.py::TestGetHonchoClient::test_local_dot_form_profile_host_uses_configured_api_key -q -o 'addopts='
  3. Run the related Honcho config tests and static checks:
    /home/seo/.hermes/hermes-agent/venv/bin/python -m pytest tests/honcho_plugin/test_client.py tests/test_honcho_client_config.py -q -o 'addopts='
    /home/seo/.hermes/hermes-agent/venv/bin/python -m ruff check plugins/memory/honcho/client.py tests/honcho_plugin/test_client.py tests/test_honcho_client_config.py
    /home/seo/.hermes/hermes-agent/venv/bin/python -m py_compile plugins/memory/honcho/client.py tests/honcho_plugin/test_client.py

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: Linux / Ubuntu 24.04-like environment

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

Screenshots / Logs

Focused regression test passed:

1 passed in 0.14s

Related Honcho tests passed:

89 passed, 17 skipped in 1.44s

Ruff passed:

All checks passed!

Compile check passed:

py_compile plugins/memory/honcho/client.py tests/honcho_plugin/test_client.py

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins tool/memory Memory tool and memory providers labels Jun 2, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: Approved

Fix for Honcho dot-form host API key resolution. The old code used config.raw.get("hosts", {}).get(config.host, {}) which didn't properly resolve dot-form host keys. Refactored to use a proper _host_block() helper. Includes a test for local dot-form profile host with configured API key.


Reviewed by Hermes Agent

…ocal-api-key

# Conflicts:
#	plugins/memory/honcho/client.py
@westkite1201

Copy link
Copy Markdown
Author

Updated this branch with the latest main and resolved the Honcho client conflict against the current SingletonSlot-based client construction. The original fix is preserved by using _host_block(...) for local-auth host lookup, so legacy dot-form host keys are still honored.\n\nLocal checks run:\n- python3 -m pytest tests/honcho_plugin/test_client.py -q\n- python3 -m py_compile plugins/memory/honcho/client.py tests/honcho_plugin/test_client.py\n- git diff --check\n- python3 scripts/check-windows-footguns.py --diff origin/main\n\nThe PR is now mergeable again on GitHub.

@ponkcore

Copy link
Copy Markdown

Confirming this bug in production — same root cause, same fix.

Setup: 2-profile self-hosted Honcho deployment (Docker, AUTH_USE_AUTH=true, baseUrl: http://localhost:8000). Default profile (hermes) + second profile (ayumi) on the same host, each with its own gateway service (hermes-gateway-ayumi.service with HERMES_HOME=~/.hermes/profiles/ayumi/).

Config: honcho.json with dot-form host keys (legacy format from before 827ce602d):

{"hosts": {"hermes": {...}, "hermes.ayumi": {"apiKey": "<JWT>", "aiPeer": "ayumi"}}}

Impact: profile_host_key("ayumi") returns hermes_ayumi (underscore), but the _is_local branch in get_honcho_client does hosts.get("hermes_ayumi") — misses the hermes.ayumi block → effective_api_key = "local" → 401 Invalid JWT on every Honcho call.

  • 679 Invalid JWT errors over 9 days (June 19–28)
  • Zero messages synced, zero observations derived for the affected profile
  • Default profile was unaffected because profile_host_key("default") returns "hermes" (no dot/underscore mismatch)

Verification of this PR's fix: Applied the same _host_block() change locally. 98/98 existing test_client.py tests pass. Regression test confirmed: without the fix, effective_api_key is "local"; with it, the correct JWT is passed to the SDK.

The fix is correct and minimal. Would be great to see this merged — it's been open since June 2 and has already caused silent data loss in at least two independent deployments (see also #36098 Issue 2).

@westkite1201

Copy link
Copy Markdown
Author

This PR still looks mergeable and has a production confirmation above. The latest head does not appear to have GitHub Actions checks attached yet. Could a maintainer approve or re-run the workflow when convenient?

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused regression fix. Current main already resolves legacy dot-form host blocks in HonchoClientConfig.from_global_config() (plugins/memory/honcho/client.py:427), but the localhost auth path bypasses that resolver with a direct lookup (plugins/memory/honcho/client.py:885) and therefore emits "local" instead of the configured key (:887).

The changed call to _host_block(...) applies the same established resolution contract at the final decision point. The added regression test captures the SDK kwargs for a hermes.profile_a legacy block and asserts the configured JWT is retained (tests/honcho_plugin/test_client.py:695 in the PR diff). No additional production client-construction path performs this localhost effective_api_key decision.

Automated hermes-sweeper review.

@alt-glitch alt-glitch added area/auth Authentication, OAuth, credential pools P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state and removed P3 Low — cosmetic, nice to have labels Jul 13, 2026
@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-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 13, 2026
@alt-glitch alt-glitch added P3 Low — cosmetic, nice to have and removed sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data P2 Medium — degraded but workaround exists labels Jul 13, 2026
@teknium1 teknium1 added the area/memory Memory subsystem: store, providers, sync, background reviews label Jul 19, 2026
@alt-glitch alt-glitch added area/config Config system, migrations, profiles and removed sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 19, 2026
@alt-glitch alt-glitch added the sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data label Aug 12, 2026
@westkite1201

Copy link
Copy Markdown
Author

Closing this PR because the requested behavior is now implemented on main, rather than because the report was invalid.

Verified at 78d338b9ee917b73468c38ba4633ed53de4942e6:

There is no remaining unique production fix to land from this branch. Thanks to everyone who supplied production confirmation and reviewed the original fix.

Updated with Hermes Agent · default profile · Telegram.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools area/config Config system, migrations, profiles area/memory Memory subsystem: store, providers, sync, background reviews comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users 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

5 participants