mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix(ci): unblock test suite + cut ~2s of dead Z.AI probes from every AIAgent
CI on main had 7 failing tests. Five were stale test fixtures; one (agent
cache spillover timeout) was covering up a real perf regression in
AIAgent construction.
The perf bug: every AIAgent.__init__ calls _check_compression_model_feasibility
→ resolve_provider_client('auto') → _resolve_api_key_provider which
iterates PROVIDER_REGISTRY. When it hits 'zai', it unconditionally calls
resolve_api_key_provider_credentials → _resolve_zai_base_url → probes 8
Z.AI endpoints with an empty Bearer token (all 401s), ~2s of pure latency
per agent, even when the user has never touched Z.AI. Landed in
9e844160 (PR for credential-pool Z.AI auto-detect) — the short-circuit
when api_key is empty was missing. _resolve_kimi_base_url had the same
shape; fixed too.
Test fixes:
- tests/gateway/test_voice_command.py: _make_adapter helpers were missing
self._voice_locks (added in PR #12644, 7 call sites — all updated).
- tests/test_toolsets.py: test_hermes_platforms_share_core_tools asserted
equality, but hermes-discord has discord_server (DISCORD_BOT_TOKEN-gated,
discord-only by design). Switched to subset check.
- tests/run_agent/test_streaming.py: test_tool_name_not_duplicated_when_resent_per_chunk
missing api_key/base_url — classic pitfall (PR #11619 fixed 16 of
these; this one slipped through on a later commit).
- tests/tools/test_discord_tool.py: TestConfigAllowlist caplog assertions
fail in parallel runs because AIAgent(quiet_mode=True) globally sets
logging.getLogger('tools').setLevel(ERROR) and xdist workers are
persistent. Autouse fixture resets the 'tools' and
'tools.discord_tool' levels per test.
Validation:
tests/cron + voice + agent_cache + streaming + toolsets + command_guards
+ discord_tool: 550/550 pass
tests/hermes_cli + tests/gateway: 5713/5713 pass
AIAgent construction without Z.AI creds: 2.2s → 0.24s (9x)
This commit is contained in:
parent
88185e7147
commit
c9b833feb3
5 changed files with 56 additions and 4 deletions
|
|
@ -353,6 +353,9 @@ def _resolve_kimi_base_url(api_key: str, default_url: str, env_override: str) ->
|
||||||
"""
|
"""
|
||||||
if env_override:
|
if env_override:
|
||||||
return env_override
|
return env_override
|
||||||
|
# No key → nothing to infer from. Return default without inspecting.
|
||||||
|
if not api_key:
|
||||||
|
return default_url
|
||||||
if api_key.startswith("sk-kimi-"):
|
if api_key.startswith("sk-kimi-"):
|
||||||
return KIMI_CODE_BASE_URL
|
return KIMI_CODE_BASE_URL
|
||||||
return default_url
|
return default_url
|
||||||
|
|
@ -480,6 +483,14 @@ def _resolve_zai_base_url(api_key: str, default_url: str, env_override: str) ->
|
||||||
if env_override:
|
if env_override:
|
||||||
return env_override
|
return env_override
|
||||||
|
|
||||||
|
# No API key set → don't probe (would fire N×M HTTPS requests with an
|
||||||
|
# empty Bearer token, all returning 401). This path is hit during
|
||||||
|
# auxiliary-client auto-detection when the user has no Z.AI credentials
|
||||||
|
# at all — the caller discards the result immediately, so the probe is
|
||||||
|
# pure latency for every AIAgent construction.
|
||||||
|
if not api_key:
|
||||||
|
return default_url
|
||||||
|
|
||||||
# Check provider-state cache for a previously-detected endpoint.
|
# Check provider-state cache for a previously-detected endpoint.
|
||||||
auth_store = _load_auth_store()
|
auth_store = _load_auth_store()
|
||||||
state = _load_provider_state(auth_store, "zai") or {}
|
state = _load_provider_state(auth_store, "zai") or {}
|
||||||
|
|
|
||||||
|
|
@ -416,6 +416,7 @@ class TestDiscordPlayTtsSkip:
|
||||||
adapter.platform = Platform.DISCORD
|
adapter.platform = Platform.DISCORD
|
||||||
adapter.config = config
|
adapter.config = config
|
||||||
adapter._voice_clients = {}
|
adapter._voice_clients = {}
|
||||||
|
adapter._voice_locks = {}
|
||||||
adapter._voice_text_channels = {}
|
adapter._voice_text_channels = {}
|
||||||
adapter._voice_sources = {}
|
adapter._voice_sources = {}
|
||||||
adapter._voice_timeout_tasks = {}
|
adapter._voice_timeout_tasks = {}
|
||||||
|
|
@ -931,6 +932,7 @@ class TestDiscordVoiceChannelMethods:
|
||||||
adapter.config = config
|
adapter.config = config
|
||||||
adapter._client = MagicMock()
|
adapter._client = MagicMock()
|
||||||
adapter._voice_clients = {}
|
adapter._voice_clients = {}
|
||||||
|
adapter._voice_locks = {}
|
||||||
adapter._voice_text_channels = {}
|
adapter._voice_text_channels = {}
|
||||||
adapter._voice_sources = {}
|
adapter._voice_sources = {}
|
||||||
adapter._voice_timeout_tasks = {}
|
adapter._voice_timeout_tasks = {}
|
||||||
|
|
@ -1712,6 +1714,7 @@ class TestVoiceTimeoutCleansRunnerState:
|
||||||
adapter.platform = Platform.DISCORD
|
adapter.platform = Platform.DISCORD
|
||||||
adapter.config = config
|
adapter.config = config
|
||||||
adapter._voice_clients = {}
|
adapter._voice_clients = {}
|
||||||
|
adapter._voice_locks = {}
|
||||||
adapter._voice_text_channels = {}
|
adapter._voice_text_channels = {}
|
||||||
adapter._voice_sources = {}
|
adapter._voice_sources = {}
|
||||||
adapter._voice_timeout_tasks = {}
|
adapter._voice_timeout_tasks = {}
|
||||||
|
|
@ -1802,6 +1805,7 @@ class TestPlaybackTimeout:
|
||||||
adapter.platform = Platform.DISCORD
|
adapter.platform = Platform.DISCORD
|
||||||
adapter.config = config
|
adapter.config = config
|
||||||
adapter._voice_clients = {}
|
adapter._voice_clients = {}
|
||||||
|
adapter._voice_locks = {}
|
||||||
adapter._voice_text_channels = {}
|
adapter._voice_text_channels = {}
|
||||||
adapter._voice_sources = {}
|
adapter._voice_sources = {}
|
||||||
adapter._voice_timeout_tasks = {}
|
adapter._voice_timeout_tasks = {}
|
||||||
|
|
@ -1983,6 +1987,7 @@ class TestVoiceChannelAwareness:
|
||||||
config.token = "fake-token"
|
config.token = "fake-token"
|
||||||
adapter = object.__new__(DiscordAdapter)
|
adapter = object.__new__(DiscordAdapter)
|
||||||
adapter._voice_clients = {}
|
adapter._voice_clients = {}
|
||||||
|
adapter._voice_locks = {}
|
||||||
adapter._voice_text_channels = {}
|
adapter._voice_text_channels = {}
|
||||||
adapter._voice_sources = {}
|
adapter._voice_sources = {}
|
||||||
adapter._voice_receivers = {}
|
adapter._voice_receivers = {}
|
||||||
|
|
@ -2453,6 +2458,7 @@ class TestVoiceTTSPlayback:
|
||||||
adapter.platform = Platform.DISCORD
|
adapter.platform = Platform.DISCORD
|
||||||
adapter.config = config
|
adapter.config = config
|
||||||
adapter._voice_clients = {}
|
adapter._voice_clients = {}
|
||||||
|
adapter._voice_locks = {}
|
||||||
adapter._voice_text_channels = {}
|
adapter._voice_text_channels = {}
|
||||||
adapter._voice_sources = {}
|
adapter._voice_sources = {}
|
||||||
adapter._voice_receivers = {}
|
adapter._voice_receivers = {}
|
||||||
|
|
@ -2633,6 +2639,7 @@ class TestUDPKeepalive:
|
||||||
adapter.platform = Platform.DISCORD
|
adapter.platform = Platform.DISCORD
|
||||||
adapter.config = config
|
adapter.config = config
|
||||||
adapter._voice_clients = {}
|
adapter._voice_clients = {}
|
||||||
|
adapter._voice_locks = {}
|
||||||
adapter._voice_text_channels = {}
|
adapter._voice_text_channels = {}
|
||||||
adapter._voice_sources = {}
|
adapter._voice_sources = {}
|
||||||
adapter._voice_receivers = {}
|
adapter._voice_receivers = {}
|
||||||
|
|
|
||||||
|
|
@ -169,6 +169,8 @@ class TestStreamingAccumulator:
|
||||||
mock_create.return_value = mock_client
|
mock_create.return_value = mock_client
|
||||||
|
|
||||||
agent = AIAgent(
|
agent = AIAgent(
|
||||||
|
api_key="test-key",
|
||||||
|
base_url="https://openrouter.ai/api/v1",
|
||||||
model="test/model",
|
model="test/model",
|
||||||
quiet_mode=True,
|
quiet_mode=True,
|
||||||
skip_context_files=True,
|
skip_context_files=True,
|
||||||
|
|
|
||||||
|
|
@ -198,12 +198,22 @@ class TestToolsetConsistency:
|
||||||
assert inc in TOOLSETS, f"{name} includes unknown toolset '{inc}'"
|
assert inc in TOOLSETS, f"{name} includes unknown toolset '{inc}'"
|
||||||
|
|
||||||
def test_hermes_platforms_share_core_tools(self):
|
def test_hermes_platforms_share_core_tools(self):
|
||||||
"""All hermes-* platform toolsets should have the same tools."""
|
"""All hermes-* platform toolsets share the same core tools.
|
||||||
|
|
||||||
|
Platform-specific additions (e.g. ``discord_server`` on
|
||||||
|
hermes-discord, gated on DISCORD_BOT_TOKEN) are allowed on top —
|
||||||
|
the invariant is that the core set is identical across platforms.
|
||||||
|
"""
|
||||||
platforms = ["hermes-cli", "hermes-telegram", "hermes-discord", "hermes-whatsapp", "hermes-slack", "hermes-signal", "hermes-homeassistant"]
|
platforms = ["hermes-cli", "hermes-telegram", "hermes-discord", "hermes-whatsapp", "hermes-slack", "hermes-signal", "hermes-homeassistant"]
|
||||||
tool_sets = [set(TOOLSETS[p]["tools"]) for p in platforms]
|
tool_sets = [set(TOOLSETS[p]["tools"]) for p in platforms]
|
||||||
# All platform toolsets should be identical
|
# All platforms must contain the shared core; platform-specific
|
||||||
for ts in tool_sets[1:]:
|
# extras are OK (subset check, not equality).
|
||||||
assert ts == tool_sets[0]
|
core = set.intersection(*tool_sets)
|
||||||
|
for name, ts in zip(platforms, tool_sets):
|
||||||
|
assert core.issubset(ts), f"{name} is missing core tools: {core - ts}"
|
||||||
|
# Sanity: the shared core must be non-trivial (i.e. we didn't
|
||||||
|
# silently let a platform diverge so far that nothing is shared).
|
||||||
|
assert len(core) > 20, f"Suspiciously small shared core: {len(core)} tools"
|
||||||
|
|
||||||
|
|
||||||
class TestPluginToolsets:
|
class TestPluginToolsets:
|
||||||
|
|
|
||||||
|
|
@ -659,6 +659,28 @@ class TestCapabilityDetection:
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
class TestConfigAllowlist:
|
class TestConfigAllowlist:
|
||||||
|
@pytest.fixture(autouse=True)
|
||||||
|
def _reset_tools_logger(self):
|
||||||
|
"""Restore the ``tools`` logger level after cross-test pollution.
|
||||||
|
|
||||||
|
``AIAgent(quiet_mode=True)`` globally sets ``tools`` and
|
||||||
|
``tools.*`` children to ``ERROR`` (see run_agent.py quiet_mode
|
||||||
|
block). xdist workers are persistent, so a streaming test on the
|
||||||
|
same worker will silence WARNING-level logs from
|
||||||
|
``tools.discord_tool`` for every test that follows. Reset here so
|
||||||
|
``caplog`` can capture warnings regardless of worker history.
|
||||||
|
"""
|
||||||
|
import logging as _logging
|
||||||
|
_prev_tools = _logging.getLogger("tools").level
|
||||||
|
_prev_dt = _logging.getLogger("tools.discord_tool").level
|
||||||
|
_logging.getLogger("tools").setLevel(_logging.NOTSET)
|
||||||
|
_logging.getLogger("tools.discord_tool").setLevel(_logging.NOTSET)
|
||||||
|
try:
|
||||||
|
yield
|
||||||
|
finally:
|
||||||
|
_logging.getLogger("tools").setLevel(_prev_tools)
|
||||||
|
_logging.getLogger("tools.discord_tool").setLevel(_prev_dt)
|
||||||
|
|
||||||
def test_empty_string_returns_none(self, monkeypatch):
|
def test_empty_string_returns_none(self, monkeypatch):
|
||||||
"""Empty config means no allowlist — all actions visible."""
|
"""Empty config means no allowlist — all actions visible."""
|
||||||
monkeypatch.setattr(
|
monkeypatch.setattr(
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue