From 2558d28a9bd90b95019a7c289db89bd0dd9f2d8e Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 14 Apr 2026 01:43:45 -0700 Subject: [PATCH] =?UTF-8?q?fix:=20resolve=20CI=20test=20failures=20?= =?UTF-8?q?=E2=80=94=20add=20missing=20functions,=20fix=20stale=20tests=20?= =?UTF-8?q?(#9483)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Production fixes: - Add clear_session_context() to hermes_logging.py (fixes 48 teardown errors) - Add clear_session() to tools/approval.py (fixes 9 setup errors) - Add SyncError M_UNKNOWN_TOKEN check to Matrix _sync_loop (bug fix) - Fall back to inline api_key in named custom providers when key_env is absent (runtime_provider.py) Test fixes: - test_memory_user_id: use builtin+external provider pair, fix honcho peer_name override test to match production behavior - test_display_config: remove TestHelpers for non-existent functions - test_auxiliary_client: fix OAuth tokens to match _is_oauth_token patterns, replace get_vision_auxiliary_client with resolve_vision_provider_client - test_cli_interrupt_subagent: add missing _execution_thread_id attr - test_compress_focus: add model/provider/api_key/base_url/api_mode to mock compressor - test_auth_provider_gate: add autouse fixture to clean Anthropic env vars that leak from CI secrets - test_opencode_go_in_model_list: accept both 'built-in' and 'hermes' source (models.dev API unavailable in CI) - test_email: verify email Platform enum membership instead of source inspection (build_channel_directory now uses dynamic enum loop) - test_feishu: add bot_added/bot_deleted handler mocks to _Builder - test_ws_auth_retry: add AsyncMock for sync_store.get_next_batch, add _pending_megolm and _joined_rooms to Matrix adapter mocks - test_restart_drain: monkeypatch-delete INVOCATION_ID (systemd sets this in CI, changing the restart call signature) - test_session_hygiene: add user_id to SessionSource - test_session_env: use relative baseline for contextvar clear check (pytest-xdist workers share context) --- gateway/platforms/matrix.py | 10 ++++++ hermes_cli/runtime_provider.py | 3 ++ hermes_logging.py | 4 +++ tests/agent/test_auxiliary_client.py | 6 ++-- tests/agent/test_compress_focus.py | 5 +++ tests/agent/test_memory_user_id.py | 24 ++++++------- tests/cli/test_cli_interrupt_subagent.py | 1 + tests/gateway/test_display_config.py | 35 ------------------- tests/gateway/test_email.py | 10 +++--- tests/gateway/test_feishu.py | 10 ++++++ tests/gateway/test_restart_drain.py | 5 ++- tests/gateway/test_session_env.py | 5 ++- tests/gateway/test_session_hygiene.py | 1 + tests/gateway/test_ws_auth_retry.py | 18 ++++++++-- tests/hermes_cli/test_auth_provider_gate.py | 7 ++++ .../test_opencode_go_in_model_list.py | 6 ++-- tools/approval.py | 11 ++++++ 17 files changed, 100 insertions(+), 61 deletions(-) diff --git a/gateway/platforms/matrix.py b/gateway/platforms/matrix.py index e38a4f947..816d88b03 100644 --- a/gateway/platforms/matrix.py +++ b/gateway/platforms/matrix.py @@ -958,6 +958,16 @@ class MatrixAdapter(BasePlatformAdapter): 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", {}) diff --git a/hermes_cli/runtime_provider.py b/hermes_cli/runtime_provider.py index 54b9ae65c..b2dec61cd 100644 --- a/hermes_cli/runtime_provider.py +++ b/hermes_cli/runtime_provider.py @@ -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 = os.getenv(key_env, "").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 = str(entry.get("api_key", "") or "").strip() if requested_norm in {ep_name, name_norm, f"custom:{name_norm}"}: # Found match by provider key diff --git a/hermes_logging.py b/hermes_logging.py index 6d611ba7c..dbef21328 100644 --- a/hermes_logging.py +++ b/hermes_logging.py @@ -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 diff --git a/tests/agent/test_auxiliary_client.py b/tests/agent/test_auxiliary_client.py index e6a9d1919..3b44cba4d 100644 --- a/tests/agent/test_auxiliary_client.py +++ b/tests/agent/test_auxiliary_client.py @@ -365,7 +365,7 @@ class TestExpiredCodexFallback: 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="sk-ant-oat-hermes-token"), \ 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() @@ -420,7 +420,7 @@ class TestExpiredCodexFallback: 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", "sk-ant-oat-cc-test-token") monkeypatch.delenv("ANTHROPIC_TOKEN", raising=False) with patch("agent.anthropic_adapter.build_anthropic_client") as mock_build: mock_build.return_value = MagicMock() @@ -786,7 +786,7 @@ class TestAuxiliaryPoolAwareness: 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" diff --git a/tests/agent/test_compress_focus.py b/tests/agent/test_compress_focus.py index a569eb9e3..8b5b1d35d 100644 --- a/tests/agent/test_compress_focus.py +++ b/tests/agent/test_compress_focus.py @@ -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 = "test-key" + compressor.api_mode = "chat_completions" return compressor diff --git a/tests/agent/test_memory_user_id.py b/tests/agent/test_memory_user_id.py index 04f90c74c..c1b82208d 100644 --- a/tests/agent/test_memory_user_id.py +++ b/tests/agent/test_memory_user_id.py @@ -109,14 +109,12 @@ class TestMemoryManagerUserIdThreading: 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", @@ -124,8 +122,10 @@ class TestMemoryManagerUserIdThreading: 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" # --------------------------------------------------------------------------- @@ -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 = "test-key" 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( diff --git a/tests/cli/test_cli_interrupt_subagent.py b/tests/cli/test_cli_interrupt_subagent.py index f4322ea6b..6821a6725 100644 --- a/tests/cli/test_cli_interrupt_subagent.py +++ b/tests/cli/test_cli_interrupt_subagent.py @@ -63,6 +63,7 @@ class TestCLISubagentInterrupt(unittest.TestCase): 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 diff --git a/tests/gateway/test_display_config.py b/tests/gateway/test_display_config.py index c9ad51280..ae2eac66e 100644 --- a/tests/gateway/test_display_config.py +++ b/tests/gateway/test_display_config.py @@ -220,41 +220,6 @@ class TestPlatformDefaults: 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 # --------------------------------------------------------------------------- diff --git a/tests/gateway/test_email.py b/tests/gateway/test_email.py index b6da07921..44e38aff4 100644 --- a/tests/gateway/test_email.py +++ b/tests/gateway/test_email.py @@ -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): diff --git a/tests/gateway/test_feishu.py b/tests/gateway/test_feishu.py index 2ef84f744..7b23a6985 100644 --- a/tests/gateway/test_feishu.py +++ b/tests/gateway/test_feishu.py @@ -631,6 +631,14 @@ class TestAdapterBehavior(unittest.TestCase): 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" @@ -654,6 +662,8 @@ class TestAdapterBehavior(unittest.TestCase): "reaction_created", "reaction_deleted", "card_action", + "bot_added", + "bot_deleted", "build", ], ) diff --git a/tests/gateway/test_restart_drain.py b/tests/gateway/test_restart_drain.py index 0c1324664..cfc2c364c 100644 --- a/tests/gateway/test_restart_drain.py +++ b/tests/gateway/test_restart_drain.py @@ -13,7 +13,10 @@ from tests.gateway.restart_test_helpers import make_restart_runner, make_restart @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( diff --git a/tests/gateway/test_session_env.py b/tests/gateway/test_session_env.py index 9f556f884..5a643a1ef 100644 --- a/tests/gateway/test_session_env.py +++ b/tests/gateway/test_session_env.py @@ -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 = runner._set_session_env(context) 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): diff --git a/tests/gateway/test_session_hygiene.py b/tests/gateway/test_session_hygiene.py index 5488296f6..325c24fac 100644 --- a/tests/gateway/test_session_hygiene.py +++ b/tests/gateway/test_session_hygiene.py @@ -374,6 +374,7 @@ async def test_session_hygiene_messages_stay_in_originating_topic(monkeypatch, t chat_id="-1001", chat_type="group", thread_id="17585", + user_id="12345", ), message_id="1", ) diff --git a/tests/gateway/test_ws_auth_retry.py b/tests/gateway/test_ws_auth_retry.py index beef6722e..0da397933 100644 --- a/tests/gateway/test_ws_auth_retry.py +++ b/tests/gateway/test_ws_auth_retry.py @@ -130,13 +130,17 @@ class TestMatrixSyncAuthRetry: 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 @@ -157,13 +161,17 @@ class TestMatrixSyncAuthRetry: 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 @@ -188,7 +196,7 @@ class TestMatrixSyncAuthRetry: 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: @@ -198,6 +206,10 @@ class TestMatrixSyncAuthRetry: 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 --git a/tests/hermes_cli/test_auth_provider_gate.py b/tests/hermes_cli/test_auth_provider_gate.py index 2eacb71be..f65ae71b8 100644 --- a/tests/hermes_cli/test_auth_provider_gate.py +++ b/tests/hermes_cli/test_auth_provider_gate.py @@ -18,6 +18,13 @@ def _write_auth_store(tmp_path, payload: dict) -> None: (hermes_home / "auth.json").write_text(json.dumps(payload, indent=2)) +@pytest.fixture(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) diff --git a/tests/hermes_cli/test_opencode_go_in_model_list.py b/tests/hermes_cli/test_opencode_go_in_model_list.py index 493d41b99..7f0815233 100644 --- a/tests/hermes_cli/test_opencode_go_in_model_list.py +++ b/tests/hermes_cli/test_opencode_go_in_model_list.py @@ -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(): diff --git a/tools/approval.py b/tools/approval.py index 70420976b..3e9ccdf75 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -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: