mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-21 16:18:55 +00:00
fix(honcho): resolve the timeout staleness check from honcho.json like the build path
The staleness check added in #66052 resolved the timeout from env, config.yaml, and the default only, while the build path also reads the honcho.json host block (timeout/requestTimeout). With a timeout configured in honcho.json, the two permanently disagreed: every no-config get_honcho_client() call — i.e. every HonchoSessionManager .honcho property access — interpreted the mismatch as a config change and tore down and rebuilt the client, defeating the singleton on the hot path it was meant to protect. Teach the check to read honcho.json through the same host-aware chain as from_global_config, memoized on the file's mtime_ns so the per-call cost stays one stat(). A genuine honcho.json timeout change is now also detected, extending #57437 to that config surface.
This commit is contained in:
parent
19bc02ff8e
commit
9769facae1
2 changed files with 98 additions and 5 deletions
|
|
@ -860,6 +860,8 @@ _cached_timeout: float | None = None
|
|||
# the staleness check on every get_honcho_client() call costs one stat()
|
||||
# instead of a full YAML load. (None, None) = not yet populated.
|
||||
_config_timeout_memo: tuple[int | None, float | None] = (None, None)
|
||||
# Same memoization for the honcho.json-derived timeout; mtime -1 = file absent.
|
||||
_honcho_json_timeout_memo: tuple[int | None, float | None] = (None, None)
|
||||
|
||||
|
||||
def _config_yaml_timeout() -> float | None:
|
||||
|
|
@ -891,11 +893,50 @@ def _config_yaml_timeout() -> float | None:
|
|||
return None
|
||||
|
||||
|
||||
def _honcho_json_timeout() -> float | None:
|
||||
"""Read timeout/requestTimeout from honcho.json (host block wins), memoized on mtime."""
|
||||
global _honcho_json_timeout_memo
|
||||
try:
|
||||
path = resolve_config_path()
|
||||
try:
|
||||
mtime_ns: int = path.stat().st_mtime_ns
|
||||
except OSError:
|
||||
mtime_ns = -1
|
||||
if _honcho_json_timeout_memo[0] == mtime_ns:
|
||||
return _honcho_json_timeout_memo[1]
|
||||
|
||||
timeout = None
|
||||
if mtime_ns != -1:
|
||||
raw = json.loads(path.read_text(encoding="utf-8"))
|
||||
host_block = _host_block(raw, resolve_active_host())
|
||||
timeout = _resolve_optional_float(
|
||||
host_block.get("timeout"),
|
||||
host_block.get("requestTimeout"),
|
||||
raw.get("timeout"),
|
||||
raw.get("requestTimeout"),
|
||||
)
|
||||
_honcho_json_timeout_memo = (mtime_ns, timeout)
|
||||
return timeout
|
||||
except Exception:
|
||||
return None
|
||||
|
||||
|
||||
def _resolve_timeout_from_sources(config: HonchoClientConfig | None) -> float:
|
||||
"""Resolve the effective timeout from env, config.yaml, and the explicit config."""
|
||||
timeout = config.timeout if config is not None else None
|
||||
if timeout is None:
|
||||
timeout = _resolve_optional_float(os.environ.get("HONCHO_TIMEOUT"))
|
||||
"""Mirror the build path's timeout resolution so the staleness check agrees with it.
|
||||
|
||||
With an explicit config this matches ``_build`` (config.timeout, then
|
||||
config.yaml, then default). With no config it matches what
|
||||
``from_global_config`` + ``_build`` would produce: honcho.json host
|
||||
block/root keys, then HONCHO_TIMEOUT, then config.yaml, then default.
|
||||
Any source skew here makes the check disagree with the built client
|
||||
forever and rebuild it on every call.
|
||||
"""
|
||||
if config is not None:
|
||||
timeout = config.timeout
|
||||
else:
|
||||
timeout = _honcho_json_timeout()
|
||||
if timeout is None:
|
||||
timeout = _resolve_optional_float(os.environ.get("HONCHO_TIMEOUT"))
|
||||
if timeout is None:
|
||||
timeout = _config_yaml_timeout()
|
||||
return timeout if timeout is not None else _DEFAULT_HTTP_TIMEOUT
|
||||
|
|
@ -1077,7 +1118,8 @@ def get_honcho_client(config: HonchoClientConfig | None = None) -> Honcho:
|
|||
|
||||
def reset_honcho_client() -> None:
|
||||
"""Reset the Honcho client singleton (useful for testing)."""
|
||||
global _cached_timeout, _config_timeout_memo
|
||||
global _cached_timeout, _config_timeout_memo, _honcho_json_timeout_memo
|
||||
_honcho_client_slot.reset()
|
||||
_cached_timeout = None
|
||||
_config_timeout_memo = (None, None)
|
||||
_honcho_json_timeout_memo = (None, None)
|
||||
|
|
|
|||
|
|
@ -773,6 +773,57 @@ class TestGetHonchoClient:
|
|||
mock_h3.assert_called_once()
|
||||
assert mock_h3.call_args.kwargs["timeout"] == 300.0
|
||||
|
||||
@pytest.mark.skipif(
|
||||
not importlib.util.find_spec("honcho"),
|
||||
reason="honcho SDK not installed"
|
||||
)
|
||||
def test_honcho_json_timeout_does_not_thrash_singleton(self, tmp_path):
|
||||
"""Regression: a timeout configured in honcho.json must not rebuild the
|
||||
client on every no-config call. The staleness check used to resolve the
|
||||
timeout without reading honcho.json, so it permanently disagreed with
|
||||
the built client and reset the singleton on each access."""
|
||||
config_file = tmp_path / "honcho.json"
|
||||
config_file.write_text(json.dumps({
|
||||
"apiKey": "cloud-key",
|
||||
"hosts": {"hermes": {"workspace": "ws", "requestTimeout": 120}},
|
||||
}))
|
||||
|
||||
fake_honcho_1 = MagicMock(name="Honcho_v1")
|
||||
fake_honcho_2 = MagicMock(name="Honcho_v2")
|
||||
|
||||
with patch("plugins.memory.honcho.client.resolve_config_path", return_value=config_file), \
|
||||
patch("hermes_cli.profiles.get_active_profile_name", return_value="default"):
|
||||
with patch("honcho.Honcho", return_value=fake_honcho_1) as mock_h1:
|
||||
client1 = get_honcho_client()
|
||||
|
||||
assert client1 is fake_honcho_1
|
||||
assert mock_h1.call_args.kwargs["timeout"] == 120.0
|
||||
|
||||
# Repeated no-config calls (the session manager hot path) must
|
||||
# return the cached client, not rebuild.
|
||||
with patch("honcho.Honcho", return_value=fake_honcho_2) as mock_h2:
|
||||
client2 = get_honcho_client()
|
||||
client3 = get_honcho_client()
|
||||
|
||||
assert client2 is fake_honcho_1
|
||||
assert client3 is fake_honcho_1
|
||||
mock_h2.assert_not_called()
|
||||
|
||||
# A real honcho.json timeout change is still detected.
|
||||
config_file.write_text(json.dumps({
|
||||
"apiKey": "cloud-key",
|
||||
"hosts": {"hermes": {"workspace": "ws", "requestTimeout": 240}},
|
||||
}))
|
||||
st = config_file.stat()
|
||||
os.utime(config_file, ns=(st.st_atime_ns, st.st_mtime_ns + 1_000_000))
|
||||
|
||||
with patch("honcho.Honcho", return_value=fake_honcho_2) as mock_h3:
|
||||
client4 = get_honcho_client()
|
||||
|
||||
assert client4 is fake_honcho_2
|
||||
mock_h3.assert_called_once()
|
||||
assert mock_h3.call_args.kwargs["timeout"] == 240.0
|
||||
|
||||
|
||||
class TestResolveSessionNameGatewayKey:
|
||||
"""Regression tests for gateway_session_key priority in resolve_session_name.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue