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.