From 9769facae11df98ea6ee5b0dc1e67f14e08a3d22 Mon Sep 17 00:00:00 2001 From: Erosika Date: Fri, 17 Jul 2026 13:23:06 -0400 Subject: [PATCH] fix(honcho): resolve the timeout staleness check from honcho.json like the build path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- plugins/memory/honcho/client.py | 52 +++++++++++++++++++++++++++--- tests/honcho_plugin/test_client.py | 51 +++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 5 deletions(-) diff --git a/plugins/memory/honcho/client.py b/plugins/memory/honcho/client.py index 19a96ba0f4a2..2e1de22d3229 100644 --- a/plugins/memory/honcho/client.py +++ b/plugins/memory/honcho/client.py @@ -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) diff --git a/tests/honcho_plugin/test_client.py b/tests/honcho_plugin/test_client.py index fc60b9370ece..0d62a72613e6 100644 --- a/tests/honcho_plugin/test_client.py +++ b/tests/honcho_plugin/test_client.py @@ -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.