mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-08 03:01:47 +00:00
Merge remote-tracking branch 'origin/main' into sid/types-and-lints
# Conflicts: # gateway/platforms/base.py # gateway/platforms/qqbot/adapter.py # gateway/platforms/slack.py # hermes_cli/main.py # scripts/batch_runner.py # tools/skills_tool.py # uv.lock
This commit is contained in:
commit
a9ed7cb3b4
117 changed files with 7791 additions and 611 deletions
|
|
@ -58,10 +58,3 @@ class TestCamofoxConfigDefaults:
|
|||
|
||||
browser_cfg = DEFAULT_CONFIG["browser"]
|
||||
assert browser_cfg["camofox"]["managed_persistence"] is False
|
||||
|
||||
def test_config_version_matches_current_schema(self):
|
||||
from hermes_cli.config import DEFAULT_CONFIG
|
||||
|
||||
# The current schema version is tracked globally; unrelated default
|
||||
# options may bump it after browser defaults are added.
|
||||
assert DEFAULT_CONFIG["_config_version"] == 20
|
||||
|
|
|
|||
|
|
@ -172,28 +172,60 @@ class TestTerminalIntegration:
|
|||
assert blocked_var not in result
|
||||
assert "PATH" in result
|
||||
|
||||
def test_passthrough_allows_blocklisted_var(self):
|
||||
from tools.environments.local import _sanitize_subprocess_env, _HERMES_PROVIDER_ENV_BLOCKLIST
|
||||
def test_passthrough_cannot_override_provider_blocklist(self):
|
||||
"""GHSA-rhgp-j443-p4rf: register_env_passthrough must NOT accept
|
||||
Hermes provider credentials — that was the bypass where a skill
|
||||
could declare ANTHROPIC_TOKEN / OPENAI_API_KEY as passthrough and
|
||||
defeat the execute_code sandbox scrubbing."""
|
||||
from tools.environments.local import (
|
||||
_sanitize_subprocess_env,
|
||||
_HERMES_PROVIDER_ENV_BLOCKLIST,
|
||||
)
|
||||
|
||||
blocked_var = next(iter(_HERMES_PROVIDER_ENV_BLOCKLIST))
|
||||
# Attempt to register — must be silently refused (logged warning).
|
||||
register_env_passthrough([blocked_var])
|
||||
|
||||
# is_env_passthrough must NOT report it as allowed
|
||||
assert not is_env_passthrough(blocked_var)
|
||||
|
||||
# Sanitizer still strips the var from subprocess env
|
||||
env = {blocked_var: "secret_value", "PATH": "/usr/bin"}
|
||||
result = _sanitize_subprocess_env(env)
|
||||
assert blocked_var in result
|
||||
assert result[blocked_var] == "secret_value"
|
||||
assert blocked_var not in result
|
||||
assert "PATH" in result
|
||||
|
||||
def test_make_run_env_passthrough(self, monkeypatch):
|
||||
from tools.environments.local import _make_run_env, _HERMES_PROVIDER_ENV_BLOCKLIST
|
||||
def test_make_run_env_blocklist_override_rejected(self):
|
||||
"""_make_run_env must NOT expose a blocklisted var to subprocess env
|
||||
even after a skill attempts to register it via passthrough."""
|
||||
import os
|
||||
from tools.environments.local import (
|
||||
_make_run_env,
|
||||
_HERMES_PROVIDER_ENV_BLOCKLIST,
|
||||
)
|
||||
|
||||
blocked_var = next(iter(_HERMES_PROVIDER_ENV_BLOCKLIST))
|
||||
monkeypatch.setenv(blocked_var, "secret_value")
|
||||
os.environ[blocked_var] = "secret_value"
|
||||
try:
|
||||
# Without passthrough — blocked
|
||||
result_before = _make_run_env({})
|
||||
assert blocked_var not in result_before
|
||||
|
||||
# Without passthrough — blocked
|
||||
result_before = _make_run_env({})
|
||||
assert blocked_var not in result_before
|
||||
# Skill tries to register it — must be refused, so still blocked
|
||||
register_env_passthrough([blocked_var])
|
||||
result_after = _make_run_env({})
|
||||
assert blocked_var not in result_after
|
||||
finally:
|
||||
os.environ.pop(blocked_var, None)
|
||||
|
||||
# With passthrough — allowed
|
||||
register_env_passthrough([blocked_var])
|
||||
result_after = _make_run_env({})
|
||||
assert blocked_var in result_after
|
||||
def test_non_hermes_api_key_still_registerable(self):
|
||||
"""Third-party API keys (TENOR_API_KEY, NOTION_TOKEN, etc.) are NOT
|
||||
Hermes provider credentials and must still pass through — skills
|
||||
that legitimately wrap third-party APIs must keep working."""
|
||||
# TENOR_API_KEY is a real example — used by the gif-search skill
|
||||
register_env_passthrough(["TENOR_API_KEY"])
|
||||
assert is_env_passthrough("TENOR_API_KEY")
|
||||
|
||||
# Arbitrary skill-specific var
|
||||
register_env_passthrough(["MY_SKILL_CUSTOM_CONFIG"])
|
||||
assert is_env_passthrough("MY_SKILL_CUSTOM_CONFIG")
|
||||
|
|
|
|||
|
|
@ -230,3 +230,102 @@ class TestEscapeDriftGuard:
|
|||
new, count, strategy, err = fuzzy_find_and_replace(content, old_string, new_string)
|
||||
assert err is None
|
||||
assert count == 1
|
||||
|
||||
|
||||
class TestFindClosestLines:
|
||||
def setup_method(self):
|
||||
from tools.fuzzy_match import find_closest_lines
|
||||
self.find_closest_lines = find_closest_lines
|
||||
|
||||
def test_finds_similar_line(self):
|
||||
content = "def foo():\n pass\ndef bar():\n return 1\n"
|
||||
result = self.find_closest_lines("def baz():", content)
|
||||
assert "def foo" in result or "def bar" in result
|
||||
|
||||
def test_returns_empty_for_no_match(self):
|
||||
content = "completely different content here"
|
||||
result = self.find_closest_lines("xyzzy_no_match_possible_!!!", content)
|
||||
assert result == ""
|
||||
|
||||
def test_returns_empty_for_empty_inputs(self):
|
||||
assert self.find_closest_lines("", "some content") == ""
|
||||
assert self.find_closest_lines("old string", "") == ""
|
||||
|
||||
def test_includes_context_lines(self):
|
||||
content = "line1\nline2\ndef target():\n pass\nline5\n"
|
||||
result = self.find_closest_lines("def target():", content)
|
||||
assert "target" in result
|
||||
|
||||
def test_includes_line_numbers(self):
|
||||
content = "line1\nline2\ndef foo():\n pass\n"
|
||||
result = self.find_closest_lines("def foo():", content)
|
||||
# Should include line numbers in format "N| content"
|
||||
assert "|" in result
|
||||
|
||||
|
||||
class TestFormatNoMatchHint:
|
||||
"""Gating tests for format_no_match_hint — the shared helper that decides
|
||||
whether a 'Did you mean?' snippet should be appended to an error.
|
||||
"""
|
||||
|
||||
def setup_method(self):
|
||||
from tools.fuzzy_match import format_no_match_hint
|
||||
self.fmt = format_no_match_hint
|
||||
|
||||
def test_fires_on_could_not_find_with_match(self):
|
||||
"""Classic no-match: similar content exists → hint fires."""
|
||||
content = "def foo():\n pass\ndef bar():\n pass\n"
|
||||
result = self.fmt(
|
||||
"Could not find a match for old_string in the file",
|
||||
0, "def baz():", content,
|
||||
)
|
||||
assert "Did you mean" in result
|
||||
assert "foo" in result or "bar" in result
|
||||
|
||||
def test_silent_on_ambiguous_match_error(self):
|
||||
"""'Found N matches' is not a missing-match failure — no hint."""
|
||||
content = "aaa bbb aaa\n"
|
||||
result = self.fmt(
|
||||
"Found 2 matches for old_string. Provide more context to make it unique, or use replace_all=True.",
|
||||
0, "aaa", content,
|
||||
)
|
||||
assert result == ""
|
||||
|
||||
def test_silent_on_escape_drift_error(self):
|
||||
"""Escape-drift errors are intentional blocks — hint would mislead."""
|
||||
content = "x = 1\n"
|
||||
result = self.fmt(
|
||||
"Escape-drift detected: old_string and new_string contain the literal sequence '\\\\''...",
|
||||
0, "x = \\'1\\'", content,
|
||||
)
|
||||
assert result == ""
|
||||
|
||||
def test_silent_on_identical_strings(self):
|
||||
"""old_string == new_string — hint irrelevant."""
|
||||
result = self.fmt(
|
||||
"old_string and new_string are identical",
|
||||
0, "foo", "foo bar\n",
|
||||
)
|
||||
assert result == ""
|
||||
|
||||
def test_silent_when_match_count_nonzero(self):
|
||||
"""If match succeeded, we shouldn't be in the error path — defense in depth."""
|
||||
result = self.fmt(
|
||||
"Could not find a match for old_string in the file",
|
||||
1, "foo", "foo bar\n",
|
||||
)
|
||||
assert result == ""
|
||||
|
||||
def test_silent_on_none_error(self):
|
||||
"""No error at all — no hint."""
|
||||
result = self.fmt(None, 0, "foo", "bar\n")
|
||||
assert result == ""
|
||||
|
||||
def test_silent_when_no_similar_content(self):
|
||||
"""Even for a valid no-match error, skip hint when nothing similar exists."""
|
||||
result = self.fmt(
|
||||
"Could not find a match for old_string in the file",
|
||||
0, "totally_unique_xyzzy_qux", "abc\nxyz\n",
|
||||
)
|
||||
assert result == ""
|
||||
|
||||
|
|
|
|||
39
tests/tools/test_image_generation_env.py
Normal file
39
tests/tools/test_image_generation_env.py
Normal file
|
|
@ -0,0 +1,39 @@
|
|||
"""FAL_KEY env var normalization (whitespace-only treated as unset)."""
|
||||
|
||||
|
||||
def test_fal_key_whitespace_is_unset(monkeypatch):
|
||||
# Whitespace-only FAL_KEY must NOT register as configured, and the managed
|
||||
# gateway fallback must be disabled for this assertion to be meaningful.
|
||||
monkeypatch.setenv("FAL_KEY", " ")
|
||||
|
||||
from tools import image_generation_tool
|
||||
|
||||
monkeypatch.setattr(
|
||||
image_generation_tool, "_resolve_managed_fal_gateway", lambda: None
|
||||
)
|
||||
|
||||
assert image_generation_tool.check_fal_api_key() is False
|
||||
|
||||
|
||||
def test_fal_key_valid(monkeypatch):
|
||||
monkeypatch.setenv("FAL_KEY", "sk-test")
|
||||
|
||||
from tools import image_generation_tool
|
||||
|
||||
monkeypatch.setattr(
|
||||
image_generation_tool, "_resolve_managed_fal_gateway", lambda: None
|
||||
)
|
||||
|
||||
assert image_generation_tool.check_fal_api_key() is True
|
||||
|
||||
|
||||
def test_fal_key_empty_is_unset(monkeypatch):
|
||||
monkeypatch.setenv("FAL_KEY", "")
|
||||
|
||||
from tools import image_generation_tool
|
||||
|
||||
monkeypatch.setattr(
|
||||
image_generation_tool, "_resolve_managed_fal_gateway", lambda: None
|
||||
)
|
||||
|
||||
assert image_generation_tool.check_fal_api_key() is False
|
||||
162
tests/tools/test_local_shell_init.py
Normal file
162
tests/tools/test_local_shell_init.py
Normal file
|
|
@ -0,0 +1,162 @@
|
|||
"""Tests for terminal.shell_init_files / terminal.auto_source_bashrc.
|
||||
|
||||
A bash ``-l -c`` invocation does NOT source ``~/.bashrc``, so tools that
|
||||
register themselves there (nvm, asdf, pyenv) stay invisible to the
|
||||
environment snapshot built by ``LocalEnvironment.init_session``. These
|
||||
tests verify the config-driven prelude that fixes that.
|
||||
"""
|
||||
|
||||
import os
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
from tools.environments.local import (
|
||||
LocalEnvironment,
|
||||
_prepend_shell_init,
|
||||
_read_terminal_shell_init_config,
|
||||
_resolve_shell_init_files,
|
||||
)
|
||||
|
||||
|
||||
class TestResolveShellInitFiles:
|
||||
def test_auto_sources_bashrc_when_present(self, tmp_path, monkeypatch):
|
||||
bashrc = tmp_path / ".bashrc"
|
||||
bashrc.write_text('export MARKER=seen\n')
|
||||
monkeypatch.setenv("HOME", str(tmp_path))
|
||||
|
||||
# Default config: auto_source_bashrc on, no explicit list.
|
||||
with patch(
|
||||
"tools.environments.local._read_terminal_shell_init_config",
|
||||
return_value=([], True),
|
||||
):
|
||||
resolved = _resolve_shell_init_files()
|
||||
|
||||
assert resolved == [str(bashrc)]
|
||||
|
||||
def test_skips_bashrc_when_missing(self, tmp_path, monkeypatch):
|
||||
# No bashrc written.
|
||||
monkeypatch.setenv("HOME", str(tmp_path))
|
||||
|
||||
with patch(
|
||||
"tools.environments.local._read_terminal_shell_init_config",
|
||||
return_value=([], True),
|
||||
):
|
||||
resolved = _resolve_shell_init_files()
|
||||
|
||||
assert resolved == []
|
||||
|
||||
def test_auto_source_bashrc_off_suppresses_default(self, tmp_path, monkeypatch):
|
||||
bashrc = tmp_path / ".bashrc"
|
||||
bashrc.write_text('export MARKER=seen\n')
|
||||
monkeypatch.setenv("HOME", str(tmp_path))
|
||||
|
||||
with patch(
|
||||
"tools.environments.local._read_terminal_shell_init_config",
|
||||
return_value=([], False),
|
||||
):
|
||||
resolved = _resolve_shell_init_files()
|
||||
|
||||
assert resolved == []
|
||||
|
||||
def test_explicit_list_wins_over_auto(self, tmp_path, monkeypatch):
|
||||
bashrc = tmp_path / ".bashrc"
|
||||
bashrc.write_text('export FROM_BASHRC=1\n')
|
||||
custom = tmp_path / "custom.sh"
|
||||
custom.write_text('export FROM_CUSTOM=1\n')
|
||||
monkeypatch.setenv("HOME", str(tmp_path))
|
||||
|
||||
# auto_source_bashrc stays True but the explicit list takes precedence.
|
||||
with patch(
|
||||
"tools.environments.local._read_terminal_shell_init_config",
|
||||
return_value=([str(custom)], True),
|
||||
):
|
||||
resolved = _resolve_shell_init_files()
|
||||
|
||||
assert resolved == [str(custom)]
|
||||
assert str(bashrc) not in resolved
|
||||
|
||||
def test_expands_home_and_env_vars(self, tmp_path, monkeypatch):
|
||||
target = tmp_path / "rc" / "custom.sh"
|
||||
target.parent.mkdir()
|
||||
target.write_text('export A=1\n')
|
||||
monkeypatch.setenv("HOME", str(tmp_path))
|
||||
monkeypatch.setenv("CUSTOM_RC_DIR", str(tmp_path / "rc"))
|
||||
|
||||
with patch(
|
||||
"tools.environments.local._read_terminal_shell_init_config",
|
||||
return_value=(["~/rc/custom.sh"], False),
|
||||
):
|
||||
resolved_home = _resolve_shell_init_files()
|
||||
|
||||
with patch(
|
||||
"tools.environments.local._read_terminal_shell_init_config",
|
||||
return_value=(["${CUSTOM_RC_DIR}/custom.sh"], False),
|
||||
):
|
||||
resolved_var = _resolve_shell_init_files()
|
||||
|
||||
assert resolved_home == [str(target)]
|
||||
assert resolved_var == [str(target)]
|
||||
|
||||
def test_missing_explicit_files_are_skipped_silently(self, tmp_path, monkeypatch):
|
||||
monkeypatch.setenv("HOME", str(tmp_path))
|
||||
with patch(
|
||||
"tools.environments.local._read_terminal_shell_init_config",
|
||||
return_value=([str(tmp_path / "does-not-exist.sh")], False),
|
||||
):
|
||||
resolved = _resolve_shell_init_files()
|
||||
|
||||
assert resolved == []
|
||||
|
||||
|
||||
class TestPrependShellInit:
|
||||
def test_empty_list_returns_command_unchanged(self):
|
||||
assert _prepend_shell_init("echo hi", []) == "echo hi"
|
||||
|
||||
def test_prepends_guarded_source_lines(self):
|
||||
wrapped = _prepend_shell_init("echo hi", ["/tmp/a.sh", "/tmp/b.sh"])
|
||||
assert "echo hi" in wrapped
|
||||
# Each file is sourced through a guarded [ -r … ] && . '…' || true
|
||||
# pattern so a missing/broken rc can't abort the bootstrap.
|
||||
assert "/tmp/a.sh" in wrapped
|
||||
assert "/tmp/b.sh" in wrapped
|
||||
assert "|| true" in wrapped
|
||||
assert "set +e" in wrapped
|
||||
|
||||
def test_escapes_single_quotes(self):
|
||||
wrapped = _prepend_shell_init("echo hi", ["/tmp/o'malley.sh"])
|
||||
# The path must survive as the shell receives it; embedded single
|
||||
# quote is escaped as '\'' rather than breaking the outer quoting.
|
||||
assert "o'\\''malley" in wrapped
|
||||
|
||||
|
||||
@pytest.mark.skipif(
|
||||
os.environ.get("CI") == "true" and not os.path.isfile("/bin/bash"),
|
||||
reason="Requires bash; CI sandbox may strip it.",
|
||||
)
|
||||
class TestSnapshotEndToEnd:
|
||||
"""Spin up a real LocalEnvironment and confirm the snapshot sources
|
||||
extra init files."""
|
||||
|
||||
def test_snapshot_picks_up_init_file_exports(self, tmp_path, monkeypatch):
|
||||
init_file = tmp_path / "custom-init.sh"
|
||||
init_file.write_text(
|
||||
'export HERMES_SHELL_INIT_PROBE="probe-ok"\n'
|
||||
'export PATH="/opt/shell-init-probe/bin:$PATH"\n'
|
||||
)
|
||||
|
||||
with patch(
|
||||
"tools.environments.local._read_terminal_shell_init_config",
|
||||
return_value=([str(init_file)], False),
|
||||
):
|
||||
env = LocalEnvironment(cwd=str(tmp_path), timeout=15)
|
||||
try:
|
||||
result = env.execute(
|
||||
'echo "PROBE=$HERMES_SHELL_INIT_PROBE"; echo "PATH=$PATH"'
|
||||
)
|
||||
finally:
|
||||
env.cleanup()
|
||||
|
||||
output = result.get("output", "")
|
||||
assert "PROBE=probe-ok" in output
|
||||
assert "/opt/shell-init-probe/bin" in output
|
||||
252
tests/tools/test_mcp_circuit_breaker.py
Normal file
252
tests/tools/test_mcp_circuit_breaker.py
Normal file
|
|
@ -0,0 +1,252 @@
|
|||
"""Tests for MCP tool-handler circuit-breaker recovery.
|
||||
|
||||
The circuit breaker in ``tools/mcp_tool.py`` is intended to short-circuit
|
||||
calls to an MCP server that has failed ``_CIRCUIT_BREAKER_THRESHOLD``
|
||||
consecutive times, then *transition back to a usable state* once the
|
||||
server has had time to recover (or an explicit reconnect succeeds).
|
||||
|
||||
The original implementation only had two states — closed and open — with
|
||||
no mechanism to transition back to closed, so a tripped breaker stayed
|
||||
tripped for the lifetime of the process. These tests lock in the
|
||||
half-open / cooldown / reconnect-resets-breaker behavior that fixes
|
||||
that.
|
||||
"""
|
||||
import json
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
pytest.importorskip("mcp.client.auth.oauth2")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _install_stub_server(mcp_tool_module, name: str, call_tool_impl):
|
||||
"""Install a fake MCP server in the module's registry.
|
||||
|
||||
``call_tool_impl`` is an async function stored at ``session.call_tool``
|
||||
(it's what the tool handler invokes).
|
||||
"""
|
||||
server = MagicMock()
|
||||
server.name = name
|
||||
session = MagicMock()
|
||||
session.call_tool = call_tool_impl
|
||||
server.session = session
|
||||
server._reconnect_event = MagicMock()
|
||||
server._ready = MagicMock()
|
||||
server._ready.is_set.return_value = True
|
||||
|
||||
mcp_tool_module._servers[name] = server
|
||||
mcp_tool_module._server_error_counts.pop(name, None)
|
||||
if hasattr(mcp_tool_module, "_server_breaker_opened_at"):
|
||||
mcp_tool_module._server_breaker_opened_at.pop(name, None)
|
||||
return server
|
||||
|
||||
|
||||
def _cleanup(mcp_tool_module, name: str) -> None:
|
||||
mcp_tool_module._servers.pop(name, None)
|
||||
mcp_tool_module._server_error_counts.pop(name, None)
|
||||
if hasattr(mcp_tool_module, "_server_breaker_opened_at"):
|
||||
mcp_tool_module._server_breaker_opened_at.pop(name, None)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Tests
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_circuit_breaker_half_opens_after_cooldown(monkeypatch, tmp_path):
|
||||
"""After a tripped breaker's cooldown elapses, the *next* call must
|
||||
actually execute against the session (half-open probe). When the
|
||||
probe succeeds, the breaker resets to fully closed.
|
||||
"""
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
|
||||
from tools import mcp_tool
|
||||
from tools.mcp_tool import _make_tool_handler
|
||||
|
||||
call_count = {"n": 0}
|
||||
|
||||
async def _call_tool_success(*a, **kw):
|
||||
call_count["n"] += 1
|
||||
result = MagicMock()
|
||||
result.isError = False
|
||||
block = MagicMock()
|
||||
block.text = "ok"
|
||||
result.content = [block]
|
||||
result.structuredContent = None
|
||||
return result
|
||||
|
||||
_install_stub_server(mcp_tool, "srv", _call_tool_success)
|
||||
mcp_tool._ensure_mcp_loop()
|
||||
|
||||
try:
|
||||
# Trip the breaker by setting the count at/above threshold and
|
||||
# stamping the open-time to "now".
|
||||
mcp_tool._server_error_counts["srv"] = mcp_tool._CIRCUIT_BREAKER_THRESHOLD
|
||||
fake_now = [1000.0]
|
||||
|
||||
def _fake_monotonic():
|
||||
return fake_now[0]
|
||||
|
||||
monkeypatch.setattr(mcp_tool.time, "monotonic", _fake_monotonic)
|
||||
# The breaker-open timestamp dict is introduced by the fix; on
|
||||
# a pre-fix build it won't exist, which will cause the test to
|
||||
# fail at the .get() inside the gate (correct — the fix is
|
||||
# required for this state to be tracked at all).
|
||||
if hasattr(mcp_tool, "_server_breaker_opened_at"):
|
||||
mcp_tool._server_breaker_opened_at["srv"] = fake_now[0]
|
||||
cooldown = getattr(mcp_tool, "_CIRCUIT_BREAKER_COOLDOWN_SEC", 60.0)
|
||||
|
||||
handler = _make_tool_handler("srv", "tool1", 10.0)
|
||||
|
||||
# Before cooldown: must short-circuit (no session call).
|
||||
result = handler({})
|
||||
parsed = json.loads(result)
|
||||
assert "error" in parsed, parsed
|
||||
assert "unreachable" in parsed["error"].lower()
|
||||
assert call_count["n"] == 0, (
|
||||
"breaker should short-circuit before cooldown elapses"
|
||||
)
|
||||
|
||||
# Advance past cooldown → next call is a half-open probe that
|
||||
# actually hits the session.
|
||||
fake_now[0] += cooldown + 1.0
|
||||
|
||||
result = handler({})
|
||||
parsed = json.loads(result)
|
||||
assert parsed.get("result") == "ok", parsed
|
||||
assert call_count["n"] == 1, "half-open probe should invoke session"
|
||||
|
||||
# On probe success the breaker must close (count reset to 0).
|
||||
assert mcp_tool._server_error_counts.get("srv", 0) == 0
|
||||
finally:
|
||||
_cleanup(mcp_tool, "srv")
|
||||
|
||||
|
||||
def test_circuit_breaker_reopens_on_probe_failure(monkeypatch, tmp_path):
|
||||
"""If the half-open probe fails, the breaker must re-arm the
|
||||
cooldown (not let every subsequent call through).
|
||||
"""
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
|
||||
from tools import mcp_tool
|
||||
from tools.mcp_tool import _make_tool_handler
|
||||
|
||||
call_count = {"n": 0}
|
||||
|
||||
async def _call_tool_fails(*a, **kw):
|
||||
call_count["n"] += 1
|
||||
raise RuntimeError("still broken")
|
||||
|
||||
_install_stub_server(mcp_tool, "srv", _call_tool_fails)
|
||||
mcp_tool._ensure_mcp_loop()
|
||||
|
||||
try:
|
||||
mcp_tool._server_error_counts["srv"] = mcp_tool._CIRCUIT_BREAKER_THRESHOLD
|
||||
fake_now = [1000.0]
|
||||
|
||||
def _fake_monotonic():
|
||||
return fake_now[0]
|
||||
|
||||
monkeypatch.setattr(mcp_tool.time, "monotonic", _fake_monotonic)
|
||||
if hasattr(mcp_tool, "_server_breaker_opened_at"):
|
||||
mcp_tool._server_breaker_opened_at["srv"] = fake_now[0]
|
||||
cooldown = getattr(mcp_tool, "_CIRCUIT_BREAKER_COOLDOWN_SEC", 60.0)
|
||||
|
||||
handler = _make_tool_handler("srv", "tool1", 10.0)
|
||||
|
||||
# Advance past cooldown, run probe, expect failure.
|
||||
fake_now[0] += cooldown + 1.0
|
||||
result = handler({})
|
||||
parsed = json.loads(result)
|
||||
assert "error" in parsed
|
||||
assert call_count["n"] == 1, "probe should invoke session once"
|
||||
|
||||
# The probe failure must have re-armed the cooldown — another
|
||||
# immediate call should short-circuit, not invoke session again.
|
||||
result = handler({})
|
||||
parsed = json.loads(result)
|
||||
assert "unreachable" in parsed.get("error", "").lower()
|
||||
assert call_count["n"] == 1, (
|
||||
"breaker should re-open and block further calls after probe failure"
|
||||
)
|
||||
finally:
|
||||
_cleanup(mcp_tool, "srv")
|
||||
|
||||
|
||||
def test_circuit_breaker_cleared_on_reconnect(monkeypatch, tmp_path):
|
||||
"""When the auth-recovery path successfully reconnects the server,
|
||||
the breaker should be cleared so subsequent calls aren't gated on a
|
||||
stale failure count — even if the post-reconnect retry itself fails.
|
||||
|
||||
This locks in the fix-#2 contract: a successful reconnect is
|
||||
sufficient evidence that the server is viable again. Under the old
|
||||
implementation, reset only happened on retry *success*, so a
|
||||
reconnect+retry-failure left the counter pinned above threshold
|
||||
forever.
|
||||
"""
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
|
||||
from tools import mcp_tool
|
||||
from tools.mcp_oauth_manager import get_manager, reset_manager_for_tests
|
||||
from mcp.client.auth import OAuthFlowError
|
||||
|
||||
reset_manager_for_tests()
|
||||
|
||||
async def _call_tool_unused(*a, **kw): # pragma: no cover
|
||||
raise AssertionError("session.call_tool should not be reached in this test")
|
||||
|
||||
_install_stub_server(mcp_tool, "srv", _call_tool_unused)
|
||||
mcp_tool._ensure_mcp_loop()
|
||||
|
||||
# Open the breaker well above threshold, with a recent open-time so
|
||||
# it would short-circuit everything without a reset.
|
||||
mcp_tool._server_error_counts["srv"] = mcp_tool._CIRCUIT_BREAKER_THRESHOLD + 2
|
||||
if hasattr(mcp_tool, "_server_breaker_opened_at"):
|
||||
import time as _time
|
||||
mcp_tool._server_breaker_opened_at["srv"] = _time.monotonic()
|
||||
|
||||
# Force handle_401 to claim recovery succeeded.
|
||||
mgr = get_manager()
|
||||
|
||||
async def _h401(name, token=None):
|
||||
return True
|
||||
|
||||
monkeypatch.setattr(mgr, "handle_401", _h401)
|
||||
|
||||
try:
|
||||
# Retry fails *after* the successful reconnect. Under the old
|
||||
# implementation this bumps an already-tripped counter even
|
||||
# higher. Under fix #2 the reset happens on successful
|
||||
# reconnect, and the post-retry bump only raises the fresh
|
||||
# count to 1 — still below threshold.
|
||||
def _retry_call():
|
||||
raise OAuthFlowError("still failing post-reconnect")
|
||||
|
||||
result = mcp_tool._handle_auth_error_and_retry(
|
||||
"srv",
|
||||
OAuthFlowError("initial"),
|
||||
_retry_call,
|
||||
"tools/call test",
|
||||
)
|
||||
# The call as a whole still surfaces needs_reauth because the
|
||||
# retry itself didn't succeed, but the breaker state must
|
||||
# reflect the successful reconnect.
|
||||
assert result is not None
|
||||
parsed = json.loads(result)
|
||||
assert parsed.get("needs_reauth") is True, parsed
|
||||
|
||||
# Post-reconnect count was reset to 0, then the failing retry
|
||||
# bumped it to exactly 1 — well below threshold.
|
||||
count = mcp_tool._server_error_counts.get("srv", 0)
|
||||
assert count < mcp_tool._CIRCUIT_BREAKER_THRESHOLD, (
|
||||
f"successful reconnect must reset the breaker below threshold; "
|
||||
f"got count={count}, threshold={mcp_tool._CIRCUIT_BREAKER_THRESHOLD}"
|
||||
)
|
||||
finally:
|
||||
_cleanup(mcp_tool, "srv")
|
||||
|
|
@ -173,6 +173,8 @@ def test_terminal_output_transform_does_not_change_approval_or_exit_code_meaning
|
|||
|
||||
|
||||
def test_terminal_output_transform_integration_with_real_plugin(monkeypatch, tmp_path):
|
||||
import yaml
|
||||
|
||||
hermes_home = Path(os.environ["HERMES_HOME"])
|
||||
plugins_dir = hermes_home / "plugins"
|
||||
plugin_dir = plugins_dir / "terminal_transform"
|
||||
|
|
@ -184,7 +186,15 @@ def test_terminal_output_transform_integration_with_real_plugin(monkeypatch, tmp
|
|||
'lambda **kw: "PLUGIN-HEAD\\n" + kw["output"] + "\\nPLUGIN-TAIL")\n',
|
||||
encoding="utf-8",
|
||||
)
|
||||
# Plugins are opt-in — must be listed in plugins.enabled to load.
|
||||
cfg_path = hermes_home / "config.yaml"
|
||||
cfg_path.write_text(
|
||||
yaml.safe_dump({"plugins": {"enabled": ["terminal_transform"]}}),
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
# Force a fresh plugin manager so the new config is picked up.
|
||||
plugins_mod._plugin_manager = plugins_mod.PluginManager()
|
||||
plugins_mod.discover_plugins()
|
||||
|
||||
long_output = "X" * 60000
|
||||
|
|
|
|||
198
tests/tools/test_tts_kittentts.py
Normal file
198
tests/tools/test_tts_kittentts.py
Normal file
|
|
@ -0,0 +1,198 @@
|
|||
"""Tests for the KittenTTS local provider in tools/tts_tool.py."""
|
||||
|
||||
import json
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import numpy as np
|
||||
import pytest
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def clean_env(monkeypatch):
|
||||
for key in ("HERMES_SESSION_PLATFORM",):
|
||||
monkeypatch.delenv(key, raising=False)
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def clear_kittentts_cache():
|
||||
"""Reset the module-level model cache between tests."""
|
||||
from tools import tts_tool as _tt
|
||||
_tt._kittentts_model_cache.clear()
|
||||
yield
|
||||
_tt._kittentts_model_cache.clear()
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def mock_kittentts_module():
|
||||
"""Inject a fake kittentts + soundfile module that return stub objects."""
|
||||
fake_model = MagicMock()
|
||||
# 24kHz float32 PCM at ~2s of silence
|
||||
fake_model.generate.return_value = np.zeros(48000, dtype=np.float32)
|
||||
fake_cls = MagicMock(return_value=fake_model)
|
||||
fake_kittentts = MagicMock()
|
||||
fake_kittentts.KittenTTS = fake_cls
|
||||
|
||||
# Stub soundfile — the real package isn't installed in CI venv, and
|
||||
# _generate_kittentts does `import soundfile as sf` at runtime.
|
||||
fake_sf = MagicMock()
|
||||
def _fake_write(path, audio, samplerate):
|
||||
# Emulate writing a real file so downstream path checks succeed.
|
||||
import pathlib
|
||||
pathlib.Path(path).write_bytes(b"RIFF\x00\x00\x00\x00WAVEfmt fake")
|
||||
fake_sf.write = _fake_write
|
||||
|
||||
with patch.dict(
|
||||
"sys.modules",
|
||||
{"kittentts": fake_kittentts, "soundfile": fake_sf},
|
||||
):
|
||||
yield fake_model, fake_cls
|
||||
|
||||
|
||||
class TestGenerateKittenTts:
|
||||
def test_successful_wav_generation(self, tmp_path, mock_kittentts_module):
|
||||
from tools.tts_tool import _generate_kittentts
|
||||
|
||||
fake_model, fake_cls = mock_kittentts_module
|
||||
output_path = str(tmp_path / "test.wav")
|
||||
result = _generate_kittentts("Hello world", output_path, {})
|
||||
|
||||
assert result == output_path
|
||||
assert (tmp_path / "test.wav").exists()
|
||||
fake_cls.assert_called_once()
|
||||
fake_model.generate.assert_called_once()
|
||||
|
||||
def test_config_passes_voice_speed_cleantext(self, tmp_path, mock_kittentts_module):
|
||||
from tools.tts_tool import _generate_kittentts
|
||||
|
||||
fake_model, _ = mock_kittentts_module
|
||||
config = {
|
||||
"kittentts": {
|
||||
"model": "KittenML/kitten-tts-mini-0.8",
|
||||
"voice": "Luna",
|
||||
"speed": 1.25,
|
||||
"clean_text": False,
|
||||
}
|
||||
}
|
||||
_generate_kittentts("Hi there", str(tmp_path / "out.wav"), config)
|
||||
|
||||
call_kwargs = fake_model.generate.call_args.kwargs
|
||||
assert call_kwargs["voice"] == "Luna"
|
||||
assert call_kwargs["speed"] == 1.25
|
||||
assert call_kwargs["clean_text"] is False
|
||||
|
||||
def test_default_model_and_voice(self, tmp_path, mock_kittentts_module):
|
||||
from tools.tts_tool import (
|
||||
DEFAULT_KITTENTTS_MODEL,
|
||||
DEFAULT_KITTENTTS_VOICE,
|
||||
_generate_kittentts,
|
||||
)
|
||||
|
||||
fake_model, fake_cls = mock_kittentts_module
|
||||
_generate_kittentts("Hi", str(tmp_path / "out.wav"), {})
|
||||
|
||||
fake_cls.assert_called_once_with(DEFAULT_KITTENTTS_MODEL)
|
||||
assert fake_model.generate.call_args.kwargs["voice"] == DEFAULT_KITTENTTS_VOICE
|
||||
|
||||
def test_model_is_cached_across_calls(self, tmp_path, mock_kittentts_module):
|
||||
from tools.tts_tool import _generate_kittentts
|
||||
|
||||
_, fake_cls = mock_kittentts_module
|
||||
_generate_kittentts("One", str(tmp_path / "a.wav"), {})
|
||||
_generate_kittentts("Two", str(tmp_path / "b.wav"), {})
|
||||
|
||||
# Same model name → class instantiated exactly once
|
||||
assert fake_cls.call_count == 1
|
||||
|
||||
def test_different_models_are_cached_separately(self, tmp_path, mock_kittentts_module):
|
||||
from tools.tts_tool import _generate_kittentts
|
||||
|
||||
_, fake_cls = mock_kittentts_module
|
||||
_generate_kittentts(
|
||||
"A", str(tmp_path / "a.wav"),
|
||||
{"kittentts": {"model": "KittenML/kitten-tts-nano-0.8-int8"}},
|
||||
)
|
||||
_generate_kittentts(
|
||||
"B", str(tmp_path / "b.wav"),
|
||||
{"kittentts": {"model": "KittenML/kitten-tts-mini-0.8"}},
|
||||
)
|
||||
|
||||
assert fake_cls.call_count == 2
|
||||
|
||||
def test_non_wav_extension_triggers_ffmpeg_conversion(
|
||||
self, tmp_path, mock_kittentts_module, monkeypatch
|
||||
):
|
||||
"""Non-.wav output path causes WAV → target ffmpeg conversion."""
|
||||
from tools import tts_tool as _tt
|
||||
|
||||
calls = []
|
||||
|
||||
def fake_shutil_which(cmd):
|
||||
return "/usr/bin/ffmpeg" if cmd == "ffmpeg" else None
|
||||
|
||||
def fake_run(cmd, check=False, timeout=None, **kw):
|
||||
calls.append(cmd)
|
||||
# Emulate ffmpeg writing the output file
|
||||
import pathlib
|
||||
out_path = cmd[-1]
|
||||
pathlib.Path(out_path).write_bytes(b"fake-mp3-data")
|
||||
return MagicMock(returncode=0)
|
||||
|
||||
monkeypatch.setattr(_tt.shutil, "which", fake_shutil_which)
|
||||
monkeypatch.setattr(_tt.subprocess, "run", fake_run)
|
||||
|
||||
output_path = str(tmp_path / "test.mp3")
|
||||
result = _tt._generate_kittentts("Hi", output_path, {})
|
||||
|
||||
assert result == output_path
|
||||
assert len(calls) == 1
|
||||
assert calls[0][0] == "/usr/bin/ffmpeg"
|
||||
|
||||
def test_missing_kittentts_raises_import_error(self, tmp_path, monkeypatch):
|
||||
"""When kittentts package is not installed, _import_kittentts raises."""
|
||||
import sys
|
||||
monkeypatch.setitem(sys.modules, "kittentts", None)
|
||||
from tools.tts_tool import _generate_kittentts
|
||||
|
||||
with pytest.raises((ImportError, TypeError)):
|
||||
_generate_kittentts("Hi", str(tmp_path / "out.wav"), {})
|
||||
|
||||
|
||||
class TestCheckKittenttsAvailable:
|
||||
def test_reports_available_when_package_present(self, monkeypatch):
|
||||
import importlib.util
|
||||
from tools.tts_tool import _check_kittentts_available
|
||||
|
||||
fake_spec = MagicMock()
|
||||
monkeypatch.setattr(
|
||||
importlib.util, "find_spec",
|
||||
lambda name: fake_spec if name == "kittentts" else None,
|
||||
)
|
||||
assert _check_kittentts_available() is True
|
||||
|
||||
def test_reports_unavailable_when_package_missing(self, monkeypatch):
|
||||
import importlib.util
|
||||
from tools.tts_tool import _check_kittentts_available
|
||||
|
||||
monkeypatch.setattr(importlib.util, "find_spec", lambda name: None)
|
||||
assert _check_kittentts_available() is False
|
||||
|
||||
|
||||
class TestDispatcherBranch:
|
||||
def test_kittentts_not_installed_returns_helpful_error(self, monkeypatch, tmp_path):
|
||||
"""When provider=kittentts but package missing, return JSON error with setup hint."""
|
||||
import sys
|
||||
monkeypatch.setitem(sys.modules, "kittentts", None)
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
|
||||
from tools.tts_tool import text_to_speech_tool
|
||||
|
||||
# Write a config telling it to use kittentts
|
||||
import yaml
|
||||
(tmp_path / "config.yaml").write_text(
|
||||
yaml.safe_dump({"tts": {"provider": "kittentts"}})
|
||||
)
|
||||
|
||||
result = json.loads(text_to_speech_tool(text="Hello"))
|
||||
assert result["success"] is False
|
||||
assert "kittentts" in result["error"].lower()
|
||||
assert "hermes setup tts" in result["error"].lower()
|
||||
|
|
@ -933,6 +933,58 @@ class TestEnableVoiceModeReal:
|
|||
assert cli._voice_mode is True
|
||||
|
||||
|
||||
class TestVoiceBeepConfigReal:
|
||||
"""Tests the CLI voice beep toggle."""
|
||||
|
||||
@patch("hermes_cli.config.load_config", return_value={"voice": {}})
|
||||
def test_beeps_enabled_by_default(self, _cfg):
|
||||
cli = _make_voice_cli()
|
||||
assert cli._voice_beeps_enabled() is True
|
||||
|
||||
@patch("hermes_cli.config.load_config", return_value={"voice": {"beep_enabled": False}})
|
||||
def test_beeps_can_be_disabled(self, _cfg):
|
||||
cli = _make_voice_cli()
|
||||
assert cli._voice_beeps_enabled() is False
|
||||
|
||||
@patch("cli._cprint")
|
||||
@patch("cli.threading.Thread")
|
||||
@patch("tools.voice_mode.play_beep")
|
||||
@patch("tools.voice_mode.create_audio_recorder")
|
||||
@patch(
|
||||
"tools.voice_mode.check_voice_requirements",
|
||||
return_value={
|
||||
"available": True,
|
||||
"audio_available": True,
|
||||
"stt_available": True,
|
||||
"details": "OK",
|
||||
"missing_packages": [],
|
||||
},
|
||||
)
|
||||
@patch(
|
||||
"hermes_cli.config.load_config",
|
||||
return_value={
|
||||
"voice": {
|
||||
"beep_enabled": False,
|
||||
"silence_threshold": 200,
|
||||
"silence_duration": 3.0,
|
||||
}
|
||||
},
|
||||
)
|
||||
def test_start_recording_skips_beep_when_disabled(
|
||||
self, _cfg, _req, mock_create, mock_beep, mock_thread, _cp
|
||||
):
|
||||
recorder = MagicMock()
|
||||
recorder.supports_silence_autostop = True
|
||||
mock_create.return_value = recorder
|
||||
mock_thread.return_value = MagicMock(start=MagicMock())
|
||||
|
||||
cli = _make_voice_cli()
|
||||
cli._voice_start_recording()
|
||||
|
||||
recorder.start.assert_called_once()
|
||||
mock_beep.assert_not_called()
|
||||
|
||||
|
||||
class TestDisableVoiceModeReal:
|
||||
"""Tests _disable_voice_mode with real CLI instance."""
|
||||
|
||||
|
|
@ -1087,6 +1139,16 @@ class TestVoiceStopAndTranscribeReal:
|
|||
cli._voice_stop_and_transcribe()
|
||||
assert cli._pending_input.empty()
|
||||
|
||||
@patch("cli._cprint")
|
||||
@patch("hermes_cli.config.load_config", return_value={"voice": {"beep_enabled": False}})
|
||||
@patch("tools.voice_mode.play_beep")
|
||||
def test_no_speech_detected_skips_beep_when_disabled(self, mock_beep, _cfg, _cp):
|
||||
recorder = MagicMock()
|
||||
recorder.stop.return_value = None
|
||||
cli = _make_voice_cli(_voice_recording=True, _voice_recorder=recorder)
|
||||
cli._voice_stop_and_transcribe()
|
||||
mock_beep.assert_not_called()
|
||||
|
||||
@patch("cli._cprint")
|
||||
@patch("cli.os.unlink")
|
||||
@patch("cli.os.path.isfile", return_value=True)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue