Skip to content

fix(honcho): extend local client timeout - #8527

Closed
DomGrieco wants to merge 1 commit into
NousResearch:mainfrom
DomGrieco:fix/honcho-local-honcho-timeout
Closed

fix(honcho): extend local client timeout#8527
DomGrieco wants to merge 1 commit into
NousResearch:mainfrom
DomGrieco:fix/honcho-local-honcho-timeout

Conversation

@DomGrieco

Copy link
Copy Markdown
Contributor

Summary

  • increase the Honcho SDK timeout to 300 seconds for local/self-hosted Honcho base URLs only
  • keep the existing default timeout behavior for cloud Honcho unchanged
  • add a focused unit test covering the local timeout override

Why

Local/self-hosted Honcho deployments can take longer than the SDK's default 60 second HTTP timeout for dialectic-style calls. When Hermes is pointed at a localhost/127.0.0.1 Honcho instance, this can cause Hermes to give up before Honcho responds even though the local server is still working.

This change keeps the scope narrow:

  • only applies when Hermes has already classified the resolved Honcho base URL as local
  • does not change cloud Honcho behavior
  • uses the Honcho SDK's existing timeout= parameter rather than introducing Hermes-specific retry logic

Test Plan

  • /Users/dgrieco/.hermes/hermes-agent/venv/bin/python -m py_compile plugins/memory/honcho/client.py tests/honcho_plugin/test_client.py
  • /Users/dgrieco/.hermes/hermes-agent/venv/bin/python -m pytest tests/honcho_plugin/test_client.py -q

@himanshikothari30

Copy link
Copy Markdown

I am still learning, but I noticed that some checks are failing. Maybe we should fix them before merging.

@DomGrieco

Copy link
Copy Markdown
Contributor Author

I am still learning, but I noticed that some checks are failing. Maybe we should fix them before merging.

These failures come from other upstream changes, not related to the changes introduced in this PR.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins labels Apr 28, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused local-Honcho fix. Current main still gives an unconfigured loopback client the generic 30-second default at plugins/memory/honcho/client.py:859-862, so the underlying local-timeout concern remains relevant.

Problems

  • The proposed unconditional timeout=300.0 assignment predates current timeout precedence. Main resolves explicit timeout settings from Honcho config/environment at plugins/memory/honcho/client.py:460-464 and config.yaml at :842-855; a salvage should not overwrite an operator-supplied local timeout.
  • The new test patches the optional honcho module without the availability guard used by neighboring construction tests at tests/honcho_plugin/test_client.py:614-617.

Suggested changes

  • Apply the loopback value only as the unresolved default, before the existing generic fallback at plugins/memory/honcho/client.py:859-862; preserve explicit values.
  • Cover loopback default, explicit loopback override, and cloud default, using the existing SDK skip convention.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages 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 12, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Superseded by configuration: after #66052 (commit b170f52) merged #57437 by @liuhao1024 and #61675 by @Elektrofussel, self-hosted users can set a long timeout in config.yaml (honcho.timeout) and it now takes effect live in long-lived processes — and local-host detection covers LAN/VPN addresses, not just localhost. That covers the slow-local-model case this PR targeted without a hardcoded blanket 300s override. Your diagnosis that local dialectic calls can exceed 60s was right and informed the direction — thanks @DomGrieco!

@teknium1 teknium1 closed this Jul 17, 2026
@teknium1 teknium1 added the area/memory Memory subsystem: store, providers, sync, background reviews label 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 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-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants