# NousResearch/hermes-agent · Broken import cleanup after BuiltinMemoryProvider removal ## Open now - Nothing open on GitHub. A cluster of issues and PRs centered on a broken test import referencing a non-existent BuiltinMemoryProvider module following dead code cleanup. Members address the import error, stale docstring references, and related CI test failures. ## TL;DR - 6 member(s): 0 open issue(s), 2 closed issue(s), 0 open PR(s), 1 merged PR(s), 3 PR(s) closed without merging. - A fix was merged to main in PR #9483. Verify it addresses your case. - **Recommended:** read PR #9483 (and any competing PRs below) before filing. Confirm the merged change covers your symptom. ## Maintainer resolution verdict - Internal-tracker verdict over this cluster's issue members: `unresolved`. This is distinct from the fix status above. Weigh it yourself. ## How this cluster was grouped - Link evidence (`STRUCTURAL_PARTIAL`): some members are connected by structural evidence (shared changed files, or a closing/superseded reference); the rest are joined by an automated relevance judgement over their text. - Members outside that structural core may belong to a different underlying problem. - Pairwise link density `0.29`: the share of member pairs that carry a link, counting a structural link in full and a relevance-only link at 0.36. `1.00` means every pair is linked. ## Closed / duplicate issues - #7574: \[Bug\]: test_memory_user_id broken import after dead code cleanup - #14402: Broken memory user_id regression test imports nonexistent agent.builtin_memory_provider module ## Prior & competing PRs - **#9483: fix: resolve CI test failures — add missing functions, fix stale tests**. merged to main · fix tier `verified` - maintainer direction: category `merged`; implication `superseded` - files: `gateway/platforms/matrix.py`, `hermes_cli/runtime_provider.py`, `hermes_logging.py`, `tests/agent/test_auxiliary_client.py`, `tests/agent/test_compress_focus.py`, `tests/agent/test_memory_user_id.py`, `tests/cli/test_cli_interrupt_subagent.py`, `tests/gateway/test_display_config.py`, `tests/gateway/test_email.py`, `tests/gateway/test_feishu.py`, `tests/gateway/test_restart_drain.py`, `tests/gateway/test_session_env.py`, `tests/gateway/test_session_hygiene.py`, `tests/gateway/test_ws_auth_retry.py`, `tests/hermes_cli/test_auth_provider_gate.py`, `tests/hermes_cli/test_opencode_go_in_model_list.py`, `tools/approval.py` - fix hunks: - `gateway/platforms/matrix.py` ```diff @@ -958,6 +958,16 @@ async def _sync_loop(self) -> None: sync_data = await client.sync( since=next_batch, timeout=30000, ) + + # nio returns SyncError objects (not exceptions) for auth + # failures like M_UNKNOWN_TOKEN. Detect and stop immediately. + _sync_msg = getattr(sync_data, "message", None) + if _sync_msg and isinstance(_sync_msg, str): + _lower = _sync_msg.lower() + if "m_unknown_token" in _lower or "unknown_token" in _lower: + logger.error("Matrix: permanent auth error from sync: %s — stopping", _sync_msg) + return + if isinstance(sync_data, dict): # Update joined rooms from sync response. rooms_join = sync_data.get("rooms", {}).get("join", {}) ``` - `hermes_cli/runtime_provider.py` ```diff @@ -287,6 +287,9 @@ def _get_named_custom_provider(requested_provider: str) -> Optional[Dict[str, An # Resolve the API key from the env var name stored in key_env key_env = str(entry.get("key_env", "") or "").strip() resolved_api_key = [redacted:secret-assignment] "").strip() if key_env else "" + # Fall back to inline api_key when key_env is absent or unresolvable + if not resolved_api_key: + resolved_api_key = [redacted:secret-assignment]"api_key", "") or "").strip() if requested_norm in {ep_name, name_norm, f"custom:{name_norm}"}: # Found match by provider key ``` - `hermes_logging.py` ```diff @@ -78,6 +78,10 @@ def set_session_context(session_id: str) -> None: _session_context.session_id = session_id +def clear_session_context() -> None: + """Clear the session ID for the current thread.""" + _session_context.session_id = None + # --------------------------------------------------------------------------- # Record factory — injects session_tag into every LogRecord at creation ``` - `tests/agent/test_auxiliary_client.py` ```diff @@ -365,7 +365,7 @@ def test_expired_codex_custom_endpoint_wins(self, tmp_path, monkeypatch): def test_hermes_oauth_file_sets_oauth_flag(self, monkeypatch): """OAuth-style tokens should get is_oauth=*** (token is not sk-ant-api-*).""" # Mock resolve_anthropic_token to return an OAuth-style token - with patch("agent.anthropic_adapter.resolve_anthropic_token", return_value="hermes-oauth-jwt-token"), \ + with patch("agent.anthropic_adapter.resolve_anthropic_token", return_value="[redacted:openai-key]"), \ patch("agent.anthropic_adapter.build_anthropic_client") as mock_build, \ patch("agent.auxiliary_client._select_pool_entry", return_value=(False, None)): mock_build.return_value = MagicMock() ``` ```diff @@ -420,7 +420,7 @@ def test_jwt_invalid_json_payload_passes_through(self, tmp_path, monkeypatch): def test_claude_code_oauth_env_sets_flag(self, monkeypatch): """CLAUDE_CODE_OAUTH_TOKEN env var should get is_oauth=True.""" - monkeypatch.setenv("CLAUDE_CODE_OAUTH_TOKEN", "cc-oauth-token-test") + monkeypatch.setenv("CLAUDE_CODE_OAUTH_TOKEN", "[redacted:openai-key]") monkeypatch.delenv("ANTHROPIC_TOKEN", raising=False) with patch("agent.anthropic_adapter.build_anthropic_client") as mock_build: mock_build.return_value = MagicMock() ``` ```diff @@ -786,7 +786,7 @@ def test_vision_auto_uses_active_provider_as_fallback(self, monkeypatch): patch("agent.anthropic_adapter.build_anthropic_client", return_value=MagicMock()), patch("agent.anthropic_adapter.resolve_anthropic_token", return_value="***"), ): - client, model = get_vision_auxiliary_client() + provider, client, model = resolve_vision_provider_client() assert client is not None assert client.__class__.__name__ == "AnthropicAuxiliaryClient" ``` - `tests/agent/test_compress_focus.py` ```diff @@ -25,6 +25,11 @@ def _make_compressor(): compressor._previous_summary = None compressor._summary_failure_cooldown_until = 0.0 compressor.summary_model = None + compressor.model = "test-model" + compressor.provider = "test" + compressor.base_url = "http://localhost" + compressor.api_key = "[redacted:secret-assignment]" + compressor.api_mode = "chat_completions" return compressor ``` - `tests/agent/test_memory_user_id.py` ```diff @@ -109,14 +109,12 @@ def test_user_id_none_not_forwarded(self): assert "user_id" not in p._init_kwargs def test_multiple_providers_all_receive_user_id(self): - from agent.builtin_memory_provider import BuiltinMemoryProvider - mgr = MemoryManager() - # Use builtin + one external (MemoryManager only allows one external) - builtin = BuiltinMemoryProvider() - ext = RecordingProvider("external") - mgr.add_provider(builtin) - mgr.add_provider(ext) + # Use one provider named "builtin" (always accepted) and one external + p1 = RecordingProvider("builtin") + p2 = RecordingProvider("external") + mgr.add_provider(p1) + mgr.add_provider(p2) mgr.initialize_all( session_id="sess-multi", ``` ```diff @@ -124,8 +122,10 @@ def test_multiple_providers_all_receive_user_id(self): user_id="slack_U12345", ) - assert ext._init_kwargs.get("user_id") == "slack_U12345" - assert ext._init_kwargs.get("platform") == "slack" + assert p1._init_kwargs.get("user_id") == "slack_U12345" + assert p1._init_kwargs.get("platform") == "slack" + assert p2._init_kwargs.get("user_id") == "slack_U12345" + assert p2._init_kwargs.get("platform") == "slack" # --------------------------------------------------------------------------- ``` ```diff @@ -211,17 +211,17 @@ class TestHonchoUserIdScoping: """Verify Honcho plugin uses gateway user_id for peer_name when provided.""" def test_gateway_user_id_overrides_peer_name(self): - """When user_id is in kwargs, cfg.peer_name should be overridden.""" + """When user_id is in kwargs and no explicit peer_name, user_id should be used.""" from plugins.memory.honcho import HonchoMemoryProvider provider = HonchoMemoryProvider() - # Create a mock config with a static peer_name + # Create a mock config with NO explicit peer_name mock_cfg = MagicMock() mock_cfg.enabled = True mock_cfg.api_key = "[redacted:secret-assignment]" mock_cfg.base_url = None - mock_cfg.peer_name = "static-user" + mock_cfg.peer_name = "" # No explicit peer_name — user_id should fill it mock_cfg.recall_mode = "tools" # Use tools mode to defer session init with patch( ``` - `tests/cli/test_cli_interrupt_subagent.py` ```diff @@ -63,6 +63,7 @@ def test_full_delegate_interrupt_flow(self): parent._delegate_depth = 0 parent._delegate_spinner = None parent.tool_progress_callback = None + parent._execution_thread_id = None # We'll track what happens with _active_children original_children = parent._active_children ``` - `tests/gateway/test_display_config.py` ```diff @@ -220,41 +220,6 @@ def test_high_tier_streaming_defaults_to_none(self): assert resolve_display_setting({}, "telegram", "streaming") is None -# --------------------------------------------------------------------------- -# get_effective_display / get_platform_defaults -# --------------------------------------------------------------------------- - -class TestHelpers: - """Helper functions return correct composite results.""" - - def test_get_effective_display_merges_correctly(self): - from gateway.display_config import get_effective_display - - config = { - "display": { - "tool_progress": "new", - "show_reasoning": True, - "platforms": { - "telegram": {"tool_progress": "verbose"}, - }, - } - } - eff = get_effective_display(config, "telegram") - assert eff["tool_progress"] == "verbose" # platform override - assert eff["show_reasoning"] is True # global - assert "tool_preview_length" in eff # default filled in - - def test_get_platform_defaults_returns_dict(self): - from gateway.display_config import get_platform_defaults - - defaults = get_platform_defaults("telegram") - assert "tool_progress" in defaults - assert "show_reasoning" in defaults - # Returns a new dict (not the shared tier dict) - defaults["tool_progress"] = "changed" - assert get_platform_defaults("telegram")["tool_progress"] != "changed" - - # --------------------------------------------------------------------------- # Config migration: tool_progress_overrides → display.platforms # --------------------------------------------------------------------------- ``` - `tests/gateway/test_email.py` ```diff @@ -334,10 +334,12 @@ class TestChannelDirectory(unittest.TestCase): """Verify email in channel directory session-based discovery.""" def test_email_in_session_discovery(self): - import gateway.channel_directory - import inspect - source = inspect.getsource(gateway.channel_directory.build_channel_directory) - self.assertIn('"email"', source) + from gateway.config import Platform + # Verify email is a Platform enum member — the dynamic loop in + # build_channel_directory iterates all Platform members, so email + # is included automatically as long as it's in the enum. + email_values = [p.value for p in Platform] + self.assertIn("email", email_values) class TestGatewaySetup(unittest.TestCase): ``` - `tests/gateway/test_feishu.py` ```diff @@ -631,6 +631,14 @@ def register_p2_card_action_trigger(self, _handler): calls.append("card_action") return self + def register_p2_im_chat_member_bot_added_v1(self, _handler): + calls.append("bot_added") + return self + + def register_p2_im_chat_member_bot_deleted_v1(self, _handler): + calls.append("bot_deleted") + return self + def build(self): calls.append("build") return "handler" ``` ```diff @@ -654,6 +662,8 @@ def builder(_encrypt_key, _verification_token): "reaction_created", "reaction_deleted", "card_action", + "bot_added", + "bot_deleted", "build", ], ) ``` - `tests/gateway/test_restart_drain.py` ```diff @@ -13,7 +13,10 @@ @pytest.mark.asyncio -async def test_restart_command_while_busy_requests_drain_without_interrupt(): +async def test_restart_command_while_busy_requests_drain_without_interrupt(monkeypatch): + # Ensure INVOCATION_ID is NOT set — systemd sets this in service mode, + # which changes the restart call signature. + monkeypatch.delenv("INVOCATION_ID", raising=False) runner, _adapter = make_restart_runner() runner.request_restart = MagicMock(return_value=True) event = MessageEvent( ``` - `tests/gateway/test_session_env.py` ```diff @@ -186,10 +186,13 @@ def test_set_session_env_includes_session_key(): session_key="tg:-1001:17585", ) + # Capture baseline value before setting (may be non-empty from another + # test in the same pytest-xdist worker sharing the context). + baseline = get_session_env("HERMES_SESSION_KEY") tokens = [redacted:secret-assignment] assert get_session_env("HERMES_SESSION_KEY") == "tg:-1001:17585" runner._clear_session_env(tokens) - assert get_session_env("HERMES_SESSION_KEY") == "" + assert get_session_env("HERMES_SESSION_KEY") == baseline def test_session_key_no_race_condition_with_contextvars(monkeypatch): ``` - `tests/gateway/test_session_hygiene.py` ```diff @@ -374,6 +374,7 @@ def _compress_context(self, messages, *_args, **_kwargs): chat_id="-1001", chat_type="group", thread_id="17585", + user_id="12345", ), message_id="1", ) ``` - `tests/gateway/test_ws_auth_retry.py` ```diff @@ -130,13 +130,17 @@ def __init__(self, message): sync_count = 0 - async def fake_sync(timeout=30000): + async def fake_sync(timeout=30000, since=None): nonlocal sync_count sync_count += 1 return SyncError("M_UNKNOWN_TOKEN: Invalid access token") adapter._client = MagicMock() adapter._client.sync = fake_sync + adapter._client.sync_store = MagicMock() + adapter._client.sync_store.get_next_batch = AsyncMock(return_value=None) + adapter._pending_megolm = [] + adapter._joined_rooms = set() async def run(): import sys ``` ```diff @@ -157,13 +161,17 @@ def test_exception_with_401_stops_loop(self): call_count = 0 - async def fake_sync(timeout=30000): + async def fake_sync(timeout=30000, since=None): nonlocal call_count call_count += 1 raise RuntimeError("HTTP 401 Unauthorized") adapter._client = MagicMock() adapter._client.sync = fake_sync + adapter._client.sync_store = MagicMock() + adapter._client.sync_store.get_next_batch = AsyncMock(return_value=None) + adapter._pending_megolm = [] + adapter._joined_rooms = set() async def run(): import types ``` ```diff @@ -188,7 +196,7 @@ def test_transient_error_retries(self): call_count = 0 - async def fake_sync(timeout=30000): + async def fake_sync(timeout=30000, since=None): nonlocal call_count call_count += 1 if call_count >= 2: ``` ```diff @@ -198,6 +206,10 @@ async def fake_sync(timeout=30000): adapter._client = MagicMock() adapter._client.sync = fake_sync + adapter._client.sync_store = MagicMock() + adapter._client.sync_store.get_next_batch = AsyncMock(return_value=None) + adapter._pending_megolm = [] + adapter._joined_rooms = set() async def run(): import types ``` - `tests/hermes_cli/test_auth_provider_gate.py` ```diff @@ -18,6 +18,13 @@ def _write_auth_store(tmp_path, payload: dict) -> None: (hermes_home / "auth.json").write_text(json.dumps(payload, indent=2)) [redacted:email](autouse=True) +def _clean_anthropic_env(monkeypatch): + """Strip Anthropic env vars so CI secrets don't leak into tests.""" + for key in ("ANTHROPIC_API_KEY", "ANTHROPIC_TOKEN", "CLAUDE_CODE_OAUTH_TOKEN"): + monkeypatch.delenv(key, raising=False) + + def test_returns_false_when_no_config(tmp_path, monkeypatch): monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes")) (tmp_path / "hermes").mkdir(parents=True, exist_ok=True) ``` - `tests/hermes_cli/test_opencode_go_in_model_list.py` ```diff @@ -16,8 +16,10 @@ def test_opencode_go_appears_when_api_key_set(): assert opencode_go is not None, "opencode-go should appear when OPENCODE_GO_API_KEY is set" assert opencode_go["models"] == ["glm-5", "kimi-k2.5", "mimo-v2-pro", "mimo-v2-omni", "minimax-m2.7", "minimax-m2.5"] - # opencode-go is in PROVIDER_TO_MODELS_DEV, so it appears as "built-in" (Part 1) - assert opencode_go["source"] == "built-in" + # opencode-go can appear as "built-in" (from PROVIDER_TO_MODELS_DEV when + # models.dev is reachable) or "hermes" (from HERMES_OVERLAYS fallback when + # the API is unavailable, e.g. in CI). + assert opencode_go["source"] in ("built-in", "hermes") def test_opencode_go_not_appears_when_no_creds(): ``` - `tools/approval.py` ```diff @@ -313,6 +313,17 @@ def disable_session_yolo(session_key: str) -> None: _session_yolo.discard(session_key) +def clear_session(session_key: str) -> None: + """Remove all approval and yolo state for a given session.""" + if not session_key: + return + with _lock: + _session_approved.pop(session_key, None) + _session_yolo.discard(session_key) + _pending.pop(session_key, None) + _gateway_queues.pop(session_key, None) + + def is_session_yolo_enabled(session_key: str) -> bool: """Return True when YOLO bypass is enabled for a specific session.""" if not session_key: ``` - **#10869: test(memory): remove dead import of non-existent BuiltinMemoryProvider**. closed without merge · fix tier `suggested` - maintainer direction: category `implemented_elsewhere`; implication `superseded`; refs `2558d28a9bd90b95019a7c289db89bd0dd9f2d8e`, `#14402` - _suggested-tier fix detail withheld; re-request with `?tier=all` to include it._ - **#7329: fix(tests): repair broken import in test_memory_user_id after dead code cleanup**. closed without merge - maintainer direction: category `implemented_elsewhere`; implication `superseded`; refs `#9483`, `#7574` - **#14446: docs(agent): remove stale BuiltinMemoryProvider references from memory module docstrings**. closed without merge - maintainer direction: category `silent_close`; implication `active` ## Fix status - Merged to main in PR #9483 (cited above). No release data is tracked. Verify downstream. - Fix last verified (merged) `2026-04-14`. This confirmation is old and may be out of date (a revert/reopen would not have invalidated it here); re-verify.