From e4cdd8d9adc51e4fe493c1509b61808b6f8100bf Mon Sep 17 00:00:00 2001 From: Erosika Date: Fri, 17 Jul 2026 15:37:56 -0400 Subject: [PATCH] fix(honcho): delegate the config.yaml timeout read to load_config_readonly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The staleness check's bespoke mtime memo keyed only on the user config.yaml, but load_config() merges the managed-scope config (HERMES_MANAGED_DIR/config.yaml, /etc/hermes) whose leaf keys win. A managed honcho.timeout with no user config.yaml made the memo cache 'no timeout' while _build resolved the managed value — the same perpetual-rebuild mismatch this PR fixes for honcho.json. A managed timeout edit was likewise invisible while the user file's mtime stayed put. load_config_readonly() is already cached on both files' signatures plus the env-ref snapshot, so use it instead of duplicating that invalidation logic; the defensive deepcopy the old memo existed to avoid is skipped by the readonly variant. Drive the rebuild test through a real config.yaml and add a HERMES_MANAGED_DIR regression test covering stable reuse and managed-timeout edits. --- plugins/memory/honcho/client.py | 35 ++++++----------- tests/honcho_plugin/test_client.py | 61 +++++++++++++++++++++++++++--- 2 files changed, 66 insertions(+), 30 deletions(-) diff --git a/plugins/memory/honcho/client.py b/plugins/memory/honcho/client.py index 2e1de22d32298..a1b6552abaa57 100644 --- a/plugins/memory/honcho/client.py +++ b/plugins/memory/honcho/client.py @@ -856,39 +856,27 @@ class HonchoClientConfig: _honcho_client_slot: SingletonSlot = SingletonSlot() _cached_timeout: float | None = None -# Memo for the config.yaml-derived timeout, keyed on the file's mtime_ns so +# Memo for the honcho.json-derived timeout, keyed on the file's mtime_ns so # 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. +# instead of a JSON parse. mtime -1 = file absent; (None, None) = not yet +# populated. config.yaml needs no such memo: load_config_readonly() is +# internally cached on both the user and managed files' signatures, and a +# bespoke key here would have to duplicate that invalidation logic. _honcho_json_timeout_memo: tuple[int | None, float | None] = (None, None) def _config_yaml_timeout() -> float | None: - """Read honcho.timeout / honcho.request_timeout from config.yaml, memoized on mtime.""" - global _config_timeout_memo + """Read honcho.timeout / honcho.request_timeout via the cached config loader.""" try: - from hermes_constants import get_hermes_home + from hermes_cli.config import load_config_readonly - cfg_path = get_hermes_home() / "config.yaml" - try: - mtime_ns: int | None = cfg_path.stat().st_mtime_ns - except OSError: - mtime_ns = None - if _config_timeout_memo[0] is not None and _config_timeout_memo[0] == mtime_ns: - return _config_timeout_memo[1] - - from hermes_cli.config import load_config - - honcho_cfg = load_config().get("honcho", {}) - timeout = None + honcho_cfg = load_config_readonly().get("honcho", {}) if isinstance(honcho_cfg, dict): - timeout = _resolve_optional_float( + return _resolve_optional_float( honcho_cfg.get("timeout"), honcho_cfg.get("request_timeout"), ) - _config_timeout_memo = (mtime_ns, timeout) - return timeout + return None except Exception: return None @@ -1118,8 +1106,7 @@ 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, _honcho_json_timeout_memo + global _cached_timeout, _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 0d62a72613e6a..f7371abd12acb 100644 --- a/tests/honcho_plugin/test_client.py +++ b/tests/honcho_plugin/test_client.py @@ -741,6 +741,11 @@ class TestGetHonchoClient: ) def test_timeout_change_triggers_client_rebuild(self): """Changing timeout config must rebuild the cached client.""" + from hermes_constants import get_hermes_home + + cfg_yaml = get_hermes_home() / "config.yaml" + cfg_yaml.write_text("honcho:\n timeout: 30\n") + fake_honcho_1 = MagicMock(name="Honcho_v1") fake_honcho_2 = MagicMock(name="Honcho_v2") cfg = HonchoClientConfig( @@ -749,30 +754,74 @@ class TestGetHonchoClient: environment="production", ) - with patch("honcho.Honcho", return_value=fake_honcho_1) as mock_h1, \ - patch("hermes_cli.config.load_config", return_value={"honcho": {"timeout": 30}}): + with patch("honcho.Honcho", return_value=fake_honcho_1) as mock_h1: client1 = get_honcho_client(cfg) assert client1 is fake_honcho_1 assert mock_h1.call_args.kwargs["timeout"] == 30.0 # Same config — should return cached client (no rebuild) - with patch("honcho.Honcho", return_value=fake_honcho_2) as mock_h2, \ - patch("hermes_cli.config.load_config", return_value={"honcho": {"timeout": 30}}): + with patch("honcho.Honcho", return_value=fake_honcho_2) as mock_h2: client2 = get_honcho_client(cfg) assert client2 is fake_honcho_1 # still cached mock_h2.assert_not_called() # Changed timeout — must rebuild - with patch("honcho.Honcho", return_value=fake_honcho_2) as mock_h3, \ - patch("hermes_cli.config.load_config", return_value={"honcho": {"timeout": 300}}): + cfg_yaml.write_text("honcho:\n timeout: 300\n") + st = cfg_yaml.stat() + os.utime(cfg_yaml, ns=(st.st_atime_ns, st.st_mtime_ns + 1_000_000)) + + with patch("honcho.Honcho", return_value=fake_honcho_2) as mock_h3: client3 = get_honcho_client(cfg) assert client3 is fake_honcho_2 # rebuilt 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_managed_config_timeout_does_not_thrash_singleton(self, tmp_path, monkeypatch): + """A managed-scope honcho.timeout with no user config.yaml must be seen + by the staleness check (stable reuse), and a managed edit must trigger + a rebuild. Regression for a memo that keyed only on the user file.""" + managed_dir = tmp_path / "managed" + managed_dir.mkdir() + managed_cfg = managed_dir / "config.yaml" + managed_cfg.write_text("honcho:\n timeout: 88\n") + monkeypatch.setenv("HERMES_MANAGED_DIR", str(managed_dir)) + + fake_honcho_1 = MagicMock(name="Honcho_v1") + fake_honcho_2 = MagicMock(name="Honcho_v2") + cfg = HonchoClientConfig( + api_key="test-key", + workspace_id="hermes", + environment="production", + ) + + with patch("honcho.Honcho", return_value=fake_honcho_1) as mock_h1: + client1 = get_honcho_client(cfg) + client2 = get_honcho_client(cfg) + + assert client1 is fake_honcho_1 + assert client2 is fake_honcho_1 + assert mock_h1.call_count == 1 + assert mock_h1.call_args.kwargs["timeout"] == 88.0 + + # A managed-timeout edit is detected (same-size write, so bump mtime). + managed_cfg.write_text("honcho:\n timeout: 99\n") + st = managed_cfg.stat() + os.utime(managed_cfg, ns=(st.st_atime_ns, st.st_mtime_ns + 1_000_000)) + + with patch("honcho.Honcho", return_value=fake_honcho_2) as mock_h2: + client3 = get_honcho_client(cfg) + + assert client3 is fake_honcho_2 + mock_h2.assert_called_once() + assert mock_h2.call_args.kwargs["timeout"] == 99.0 + @pytest.mark.skipif( not importlib.util.find_spec("honcho"), reason="honcho SDK not installed"